code-423n4 / 2024-01-opus-findings

0 stars 0 forks source link

overflow exist at assert_can_enter in sentinel.cairo can cause withdraw too many exceed max_amt #218

Closed c4-bot-2 closed 9 months ago

c4-bot-2 commented 10 months ago

Lines of code

https://github.com/code-423n4/2024-01-opus/blob/4720e9481a4fb20f4ab4140f9cc391a23ede3817/src/core/sentinel.cairo#L288 https://github.com/code-423n4/2024-01-opus/blob/4720e9481a4fb20f4ab4140f9cc391a23ede3817/src/core/abbot.cairo#L203

Vulnerability details

Impact

This vulnerability could allow an attacker to bypass the intended check on the maximum asset amount, potentially leading to unauthorized access or manipulation of assets within the contract

Proof of Concept

The vulnerability can be demonstrated by providing input values that cause an integer overflow in the addition operation within the assert_can_enter function. By carefully crafting the input values, an attacker could make the sum of 'current_total' and 'enter_amt' exceed the maximum value representable by the data type used, thus bypassing the intended check on the maximum asset amount

call: withdraw(https://github.com/code-423n4/2024-01-opus/blob/4720e9481a4fb20f4ab4140f9cc391a23ede3817/src/core/abbot.cairo#L203) ---->convert_to_yang(https://github.com/code-423n4/2024-01-opus/blob/4720e9481a4fb20f4ab4140f9cc391a23ede3817/src/core/abbot.cairo#L210)

The potential overflow issue in the assert_can_enter function, particularly when dealing with large yang_amt. To ensure that the addition operation doesn't result in an overflow

Tools Used

Manual review

Recommended Mitigation Steps

assert(max_amt >= enter_amt, "SE: Exceeds max amount allowed");
assert(current_total <= max_amt - enter_amt, "SE: Exceeds max amount allowed");

Assessed type

Under/Overflow

tserg commented 10 months ago

It is not possible for current_total + enter_amt to overflow the primitive u128 type without throwing an error.

c4-pre-sort commented 9 months ago

bytes032 marked the issue as insufficient quality report

alex-ppg commented 9 months ago

The Warden details how an overflow can occur in the u128 data type of Cairo; this statement is invalid as the u128 primitive is a protected type and will handle such instances with an error. Only the felt data type is prone to such overflows, and the Warden has not highlighted an instance that uses it rendering this exhibit invalid.

c4-judge commented 9 months ago

alex-ppg marked the issue as unsatisfactory: Invalid