Lines of code

https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L185-L189

Vulnerability details

Impact

This can allow incorrect delegation check results to be returned if there was an underlying error in the check logic.

Proof of Concept

This is used in the checkDelegate functions after checking the delegation validity and storing the result in the valid variable. Normally, Solidity would copy the valid bool to memory, add padding to 32 bytes, and return that. The assembly code here skips that by directly writing the cleaned valid value to memory scratch space and returning just that 32 bytes. The issue is that if there was an error or invalid opcode during the delegation check, Solidity would bubble that up and revert the whole call. But with the direct assembly return, any errors are silenced and it will just return an undefined valid value. This could allow incorrect delegation check results to be returned if there was an underlying error in the check logic.

Tools Used

Manual

Recommended Mitigation Steps

The assembly return should be wrapped in a try/catch to handle any errors

Assessed type

Other

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

The assembly optimization at the end of the contract does skip Solidity's safety checks and could potentially hide error conditions. #250