code-423n4 / 2023-05-particle-findings

0 stars 0 forks source link

QA Report #28

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

See the markdown file with the details of this report here.

c4-judge commented 1 year ago

hansfriese marked the issue as grade-a

c4-sponsor commented 1 year ago

wukong-particle marked the issue as sponsor acknowledged

wukong-particle commented 1 year ago

NC-1, Current compiler preference is ^0.8.17, so 0.8.17 hasn't yet supported this NC-2, Acknowledged, explicit return for readability NC-3, Used uint256 instead of bool for better gas

wukong-particle commented 1 year ago

L-1, will likely use 0.8.19, this duplicates the slither findings in https://github.com/code-423n4/2023-05-particle/tree/main/slither#pragma L-2, IIUC, same as https://github.com/code-423n4/2023-05-particle-findings/issues/9 L-3, we impose strict wei-level check to ensure no leaky bucket. Extra fees should be included in the "amount" argument. L-4, understood, but this is a designed case where anyone can supply NFT using this push-based method via onERC721Received, the "arbitrary" lender specified by "from" is as if this "from" address calls "supplyNft" directly, should be a benign behavior L-5, understood, this push-based method should only be used by our front-end, we are not liable for the accidental loss via direct interaction with the contract L-6, acknowledged, will use this suggestion L-7, acknowledged, will consider using Ownable2Step L-8, acknowledged, will provide a safer check L-9, understood, this should be allowed because this is a pricy attack as circulating nft in real market has a bid-ask spread that the attacker needs to fill L-10, understood, will consider the function check, though the check before/after the arbitrary function call should strictly prevent all mis-use cases

hansfriese commented 1 year ago

All findings are valid.

L10 N3

hansfriese commented 1 year ago

22, #23 were downgraded to LOW,


L12 N3

Marking the best

c4-judge commented 1 year ago

hansfriese marked the issue as selected for report

wukong-particle commented 1 year ago

L1 fixed with https://github.com/Particle-Platforms/particle-exchange-protocol/pull/4

wukong-particle commented 1 year ago

L2 mitigated with https://github.com/Particle-Platforms/particle-exchange-protocol/pull/12 L6 fixed with https://github.com/Particle-Platforms/particle-exchange-protocol/pull/8 L8 fixed with https://github.com/Particle-Platforms/particle-exchange-protocol/pull/12 L9 similar to https://github.com/code-423n4/2023-05-particle-findings/issues/44#issuecomment-1585012812

NC1 fixed with https://github.com/Particle-Platforms/particle-exchange-protocol/commit/66b29d522600f54325e307cca77975def925f920 NC2 fixed with https://github.com/Particle-Platforms/particle-exchange-protocol/pull/18 NC3 not binary toggle, in the future we may allow other WETH equivalent tokens