search

code-423n4 / 2022-12-caviar-findings

2 stars 1 forks source link

Invalid tokens can be added to the pair #493

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/outdoteth/caviar/blob/main/src/Pair.sol#L477

Vulnerability details

Impact

merkleRoot is a bytes32 and it is compared to bytes23(0) which makes it possible for a non-zero merkleRoot to be set in the constructor and yet still all tokens will be declared as valid

Proof of Concept

https://github.com/outdoteth/caviar/blob/main/src/Pair.sol#L477

Tools Used

Manual Audit

Recommended Mitigation Steps

compare merkleRoot to bytes32(0)

iFrostizz commented 1 year ago

If the merkleRoot was not set to 0 in the constructor, then the condition would evaluate to false and check for the leaf of the merkle tree rather than returning early.

iFrostizz commented 1 year ago

got it. it's compared to a "bytes23". For null values, bytes32(0) is equivalent to bytes23(0). This should be more of a QA though.

Shungy commented 1 year ago

Yes, I added this as QA: https://github.com/code-423n4/2022-12-caviar-findings/blob/main/data/shung-Q.md#low-4-incorrect-type-is-used

There is no risk, bytes23(0) will still be zero, and will properly check non-zero merkle root.

I think this finding must be invalidated due to low quality report and inflated severity.

c4-judge commented 1 year ago

berndartmueller marked the issue as unsatisfactory: Insufficient quality