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

0 stars 0 forks source link

Long Revert Strings #16

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

ye0lde

Vulnerability details

Impact

Shortening revert strings to fit in 32 bytes will decrease deploy 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-ambire/blob/bc01af4df3f70d1629c4e22a72c19e6a814db70d/contracts/libs/SignatureValidatorV2.sol#L33 https://github.com/code-423n4/2021-10-ambire/blob/bc01af4df3f70d1629c4e22a72c19e6a814db70d/contracts/libs/SignatureValidatorV2.sol#L55 https://github.com/code-423n4/2021-10-ambire/blob/bc01af4df3f70d1629c4e22a72c19e6a814db70d/contracts/libs/SignatureValidatorV2.sol#L59 https://github.com/code-423n4/2021-10-ambire/blob/bc01af4df3f70d1629c4e22a72c19e6a814db70d/contracts/libs/SignatureValidatorV2.sol#L64 https://github.com/code-423n4/2021-10-ambire/blob/bc01af4df3f70d1629c4e22a72c19e6a814db70d/contracts/libs/SignatureValidatorV2.sol#L65

Tools Used

Visual Studio Code, Remix

Recommended Mitigation Steps

Shorten the revert strings to fit in 32 bytes.

Ivshti commented 3 years ago

resolved in https://github.com/AmbireTech/adex-protocol-eth/commit/4e7c214a360ee4b685424e90d9c881c2b87cb373

GalloDaSballo commented 3 years ago

Agree with the finding, brownie handles this via // dev comments, not sure if hardhat provides a similar alternative (that costs 0)

GalloDaSballo commented 3 years ago

The sponsor has applied the improvement by using shorter strings