search

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

0 stars 0 forks source link

Safe some gas on the nonce increment #14

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

gpersoon

Vulnerability details

Impact

The increment of the nonce in function execute() of Identity.sol uses a temporary variable.

This is not necessary because the temporary variable isn't used elsewhere. Thus a bit of gas could be saved by using nonce++ Note this is also used in the contract QuickAccManager.sol

Proof of Concept

https://github.com/code-423n4/2021-10-ambire/blob/bc01af4df3f70d1629c4e22a72c19e6a814db70d/contracts/Identity.sol#L91-L95

https://github.com/code-423n4/2021-10-ambire/blob/bc01af4df3f70d1629c4e22a72c19e6a814db70d/contracts/wallet/QuickAccManager.sol#L64

Tools Used

Recommended Mitigation Steps

Use "nonce++" in the following way: bytes32 hash = keccak256(abi.encode(address(this), block.chainid, nonce++, txns));

Ivshti commented 2 years ago

Not sure if we should fix this cause it takes away some of the readability

Ivshti commented 2 years ago

Fixed in QuickAccManager, as for Identity - fixing it there would open a reentrancy vulnr. Marking this as resolved

GalloDaSballo commented 2 years ago

Agree with the finding, the sponsor has mitigated both findings

The first one (QuickAccManager) directly

The other one (Identity) via a different suggestion