code-423n4 / 2021-10-tempus-findings

0 stars 0 forks source link

Long Revert Strings #28

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

ye0lde

Vulnerability details

Impact

Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition has been met.

Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.

Proof of Concept

Revert strings > 32 bytes are here: https://github.com/code-423n4/2021-10-tempus/blob/63f7639aad08f2bba717830ed81e0649f7fc23ee/contracts/TempusController.sol#L445 https://github.com/code-423n4/2021-10-tempus/blob/63f7639aad08f2bba717830ed81e0649f7fc23ee/contracts/TempusController.sol#L473 https://github.com/code-423n4/2021-10-tempus/blob/63f7639aad08f2bba717830ed81e0649f7fc23ee/contracts/TempusController.sol#L500 https://github.com/code-423n4/2021-10-tempus/blob/63f7639aad08f2bba717830ed81e0649f7fc23ee/contracts/TempusController.sol#L597 https://github.com/code-423n4/2021-10-tempus/blob/63f7639aad08f2bba717830ed81e0649f7fc23ee/contracts/TempusPool.sol#L103 https://github.com/code-423n4/2021-10-tempus/blob/63f7639aad08f2bba717830ed81e0649f7fc23ee/contracts/TempusPool.sol#L157 https://github.com/code-423n4/2021-10-tempus/blob/63f7639aad08f2bba717830ed81e0649f7fc23ee/contracts/TempusPool.sol#L176 https://github.com/code-423n4/2021-10-tempus/blob/63f7639aad08f2bba717830ed81e0649f7fc23ee/contracts/TempusPool.sol#L273 https://github.com/code-423n4/2021-10-tempus/blob/63f7639aad08f2bba717830ed81e0649f7fc23ee/contracts/amm/TempusAMM.sol#L127 https://github.com/code-423n4/2021-10-tempus/blob/63f7639aad08f2bba717830ed81e0649f7fc23ee/contracts/pools/AaveTempusPool.sol#L53 https://github.com/code-423n4/2021-10-tempus/blob/63f7639aad08f2bba717830ed81e0649f7fc23ee/contracts/pools/CompoundTempusPool.sol#L49-L51 https://github.com/code-423n4/2021-10-tempus/blob/63f7639aad08f2bba717830ed81e0649f7fc23ee/contracts/pools/LidoTempusPool.sol#L44 https://github.com/code-423n4/2021-10-tempus/blob/63f7639aad08f2bba717830ed81e0649f7fc23ee/contracts/pools/LidoTempusPool.sol#L61

Tools Used

Visual Studio Code, Remix

Recommended Mitigation Steps

Shorten the revert strings to fit in 32 bytes.

Or in contracts using solc version 0.8.4 or greater use the Custom Errors feature. Or as you've done in TempusAMM.sol use your own custom error implementation (Errors.BPT_OUT_MIN_AMOUNT).

mijovic commented 2 years ago

Fair point. We are aware of this, and we plan to implement custom errors in TempusPool and TempusController

RedFox20 commented 2 years ago

Will not be doing this fix at this point, in favor of keeping readable error messages. We were aware of this previously and planned to mitigate in later version instead.