code-423n4 / 2023-09-delegate-findings

2 stars 1 forks source link

Dirty boolean value other than 0/1 could be manipulated by an attacker when returned from _validateFrom. #248

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L471-L473 https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L167-L174

Vulnerability details

Impact

An attacker could potentially exploit this to bypass delegation checks if they could manipulate the return value of _validateFrom to be a dirty boolean.

Proof of Concept

The key parts of the code related to this potential issue are:

  1. The _validateFrom function returns a bool indicating if a delegation is valid based on the from address:https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L471-L473
  2. The check functions like checkDelegateForAll call _validateFrom and return the bool directly via assembly:https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L167-L174
  3. The assembly converts the bool valid to 0/1 by first negating it with iszero and then negating again with a second iszero. The potential issue is that a dirty boolean value other than 0/1 could be manipulated by an attacker when returned from _validateFrom. This dirty value would then be incorrectly converted to a clean 0/1 value in the assembly return.

For example, an attacker could potentially find a way to make _validateFrom return 0xff instead of the correct 0x01. The assembly would convert this to 0x01 incorrectly.

Tools Used

Manual

Recommended Mitigation Steps

The contract should ensure bools are properly cleaned before the assembly return conversion

Assessed type

Other

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Invalid