Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Long Revert Strings #16

Open
code423n4 opened this issue Oct 17, 2021 · 3 comments
Open

Long Revert Strings #16

code423n4 opened this issue Oct 17, 2021 · 3 comments
Labels
bug Warden finding G (Gas Optimization) resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@code423n4
Copy link
Contributor

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.

code423n4 added a commit that referenced this issue Oct 17, 2021
@Ivshti Ivshti added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Oct 19, 2021
@Ivshti
Copy link
Member

Ivshti commented Oct 20, 2021

resolved in AmbireTech/adex-protocol-eth@4e7c214

@Ivshti Ivshti added the resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) label Oct 20, 2021
@GalloDaSballo
Copy link
Collaborator

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

@GalloDaSballo
Copy link
Collaborator

The sponsor has applied the improvement by using shorter strings

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Warden finding G (Gas Optimization) resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

3 participants