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

transferFrom gas improvement #187

Open
code423n4 opened this issue Feb 2, 2022 · 1 comment
Open

transferFrom gas improvement #187

code423n4 opened this issue Feb 2, 2022 · 1 comment
Labels
bug Something isn't working 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

sirhashalot

Vulnerability details

Impact

The ERC20Burnable.sol file has code copied from the OpenZeppelin ERC20.sol contract. The Behodler code transferFrom() function does use the latest version of the OpenZeppelin code, modified earlier in Jan 2022 in PR 3085, which can save gas if currentAllowance == type(uint256).max.

A second gas savings that has been present in OpenZeppelin for some time but is not in the Behodler code is to add an unchecked clause around the approve() call.

Proof of Concept

The Behodler transferFrom() function doesn't use the latest edits from OZ or the unchecked clause on the approve call. In contrast, the OZ code does use these edits for gas savings.

Recommended Mitigation Steps

Use the latest OZ edits and the unchecked clause for gas savings if it doesn't introduce overflow or underflow conditions.

@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Feb 2, 2022
code423n4 added a commit that referenced this issue Feb 2, 2022
@gititGoro gititGoro added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Feb 5, 2022
@gititGoro gititGoro added the unresolved indicate confirmed issues that haven't been resolved with a PR label Feb 12, 2022
@gititGoro gititGoro added resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) and removed unresolved indicate confirmed issues that haven't been resolved with a PR labels Jul 4, 2022
@gititGoro
Copy link
Collaborator

Behodler/limbo#33

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working 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

2 participants