"rights" stored in memory is overwriting the memory block storing "from" and 32 bytes memory is given to store 20 byes long "contract_"

[c4-submissions]

Lines of code

https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/libraries/RegistryHashes.sol#L170-L181 https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/libraries/RegistryHashes.sol#L195-L209 https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/libraries/RegistryHashes.sol#L272-L283 https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/libraries/RegistryHashes.sol#L297-L312

Vulnerability details

Impact

Detailed description of the impact of this finding.

Expected code should keccak over packed encoding of (rights, from, to, contract_, tokenId) but as 'rights' values are overwriting 'from' values and 32 bytes memory block has been allocated to 'contract_' which is 20 bytes long so rest 12 bytes contain arbitrary values hence values available for encoding is not as expected.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/libraries/RegistryHashes.sol#L170-L181 https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/libraries/RegistryHashes.sol#L195-L209 https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/libraries/RegistryHashes.sol#L272-L283 https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/libraries/RegistryHashes.sol#L297-L312

mstore(add(ptr, 20), from) will store 'from' value from 20th position but 'rights' being 32 bytes long mstore(ptr, rights) will store 'rights' from 0th byte position till 31st byte position hence overwriting 'from' value in the memory. Also mstore(add(ptr, 60), contract_) is storing 'contract' which is 20 bytes long from ptr+60 and storing the next variable 'tokenId' here mstore(add(ptr, 92), tokenId) from ptr + 92 leaving 12 bytes space in between which contain arbitrary values.

Tools Used

Manual review

Recommended Mitigation Steps

Carefully consider sizes of variables like in https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/libraries/RegistryHashes.sol#L170-L181 code can be modified as mstore(add(ptr, 92), tokenId) mstore(add(ptr, 72), contract_) mstore(add(ptr, 52), to) mstore(add(ptr, 32), from) mstore(ptr, rights)

Assessed type

en/de-code

[GalloDaSballo]

Should have sent a small POC showing the invalid behavioiur

[c4-judge]

GalloDaSballo marked the issue as duplicate of #112

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Invalid