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

0 stars 0 forks source link

IdentityFactory.withdraw can be external #10

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

loop

Vulnerability details

The withdraw function in IdentityFactory.sol is declared as public but can be external since it is not used internally.

Impact

Saves some gas in case it ever needs to be called.

Proof of Concept

https://github.com/code-423n4/2021-10-ambire/blob/main/contracts/IdentityFactory.sol#L52

Ivshti commented 3 years ago

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

GalloDaSballo commented 3 years ago

Any function not used internally is best set to external Here's a neat SO discussion that shows some of the gas math: https://ethereum.stackexchange.com/questions/19380/external-vs-public-best-practices

GalloDaSballo commented 3 years ago

The sponsor has applied the recommendation