ArkProject: NFT Bridge
ArkProject
NFTBridge
60,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

of by one error in `Cairo::isFelt(...)`

audinarey

Summary

The Cairo::isFelt(...) function is used extensively in the codebase to ensure values are properly contained in the cairo felt256 value. The maximum value of any felt256variable is SN_MODULUS shown below

SN_MODULUS =
3618502788666131213697322783095070105623107215331596699973092056135872020481;

Vulnerability Details

However, as shown in the function below on L58, if valis the exactly SN_MODULUS1the function will return false and this could cause functions like

  • Brdge::depositTokens(...)

  • snaddressWrap(...)and felt252Wrap(...) (which on turn are used in the Protocoland Statecontracts respectively)

to revert and fail

File: Cairo.sol
51: function isFelt252(
52: uint256 val
53: )
54: internal
55: pure
56: returns (bool)
57: {
58: >> return val < SN_MODULUS; // @audit off by one error SUGG: change from < to <=
59: }
​

This leads to a DOS in the most the contracts where the function is used

Impact

This breaks core contract functionality and could lead to a DOS

Tools Used

Manual review

Recommendations

Modifiy theCairo::isFelt(...)function as shown below

File: Cairo.sol
51: function isFelt252(
52: uint256 val
53: )
54: internal
55: pure
56: returns (bool)
57: {
-58: return val < SN_MODULUS;
+58: return val <= SN_MODULUS;
59: }
​
Updates

Lead Judging Commences

n0kto Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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