search

code-423n4 / 2023-09-maia-findings

25 stars 17 forks source link

BranchPort._checkTimeLimit reverts with an integer underflow, custom error message could be more helpful #470

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-maia/blob/f5ba4de628836b2a29f9b5fff59499690008c463/src/BranchPort.sol#L493

Vulnerability details

Impact

BranchPort._checkTimeLimit() reverts with a genaric overflow/underflow error @ BranchPort#L493 when a user tries to manage() @ #L152 their tokens. This may lead to user frustration when using the protocol if users are not tracking their daily limits and may affect user adoption.

POC

Update testManageExceedsDailyLimit by commenting out vm.expectRevert() line @ RootForkTest.t.sol#L1498

Run test:

forge test --match-test testManageExceedsDailyLimit

Reverts with generic Arithmetic over/underflow error

Failing tests:
Encountered 1 failing test in test/ulysses-omnichain/RootForkTest.t.sol:RootForkTest
[FAIL. Reason: Arithmetic over/underflow] testManageExceedsDailyLimit() (gas: 3032225)

Recommended Mitigation Steps

Consider adding a more verbose revert() statement for end users to let them know they have exceeded their daily limit. This is a quality of life enchancement for end users that should help with user adoption (i.e.: reduce end user frustration).

Assessed type

Error

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as low quality report

0xA5DF commented 1 year ago

Better error messages is QA

c4-judge commented 1 year ago

alcueca marked the issue as unsatisfactory: Overinflated severity