Storage Proofs
Curve
DeFiLayer 1Layer 2
14,723 OP
View results
Previous Next
Submission Details
Severity: low
Invalid

Unchecked return value of external calls (ScrvusdVerifierV2.sol)

simranjitsinghss

Summary:

Hi,

I have found out a potential vulnerability in the contract 'ScrvusdVerifierV2.sol' in which the contract doesn't check the return value for the external call to update update_profit_max_unlock_time.

Vulnerability Details:

The key details of this potential bug can be given as follows:

In the function verifyPeriodByBlockHash, the contract doesn't check return value of the external call to update_profit_max_unlock_time. Which means if external call fails (due to revert in oracle contract), the transaction will still succeed but leads to inconsistent state about the update.

Impact:

Leads to failures like period is not updated in the oracle, but the caller assumes it was.

Tools Used:

Manual Code Analysis

Recommendations:

Add require statement to check if external call succeeds:

function verifyPeriodByBlockHash(
bytes memory _block_header_rlp,
bytes memory _proof_rlp
) external returns (bool) {
Verifier.BlockHeader memory block_header = Verifier.parseBlockHeader(_block_header_rlp);
require(block_header.hash != bytes32(0), "Invalid blockhash");
require(
block_header.hash == IBlockHashOracle(ScrvusdVerifierV1.BLOCK_HASH_ORACLE).get_block_hash(block_header.number),
"Blockhash mismatch"
);
​
uint256 period = _extractPeriodFromProof(block_header.stateRootHash, _proof_rlp);
@> bool success = IScrvusdOracleV2(SCRVUSD_ORACLE).update_profit_max_unlock_time(period, block_header.number);
require(success, "Failed to update oracle");
return true;
}
Updates

Lead Judging Commences

0xnevi Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

[invalid] finding-verifyPeriodByStateRoot-return-value

Non-acceptable severity, given there is arguably no impact here. If the verification function reverts, then the block number must have been outdated, which aligns with preventing updating of the max unlock time. The `verifyScrvusdByStateRoot` is unused with regard to in-scope contract context, so there is no evidence a boolean return variable is compulsory

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.