Platform: Code4rena
Start Date: 05/10/2022
Pot Size: $50,000 USDC
Total HM: 2
Participants: 80
Period: 5 days
Judge: GalloDaSballo
Id: 168
League: ETH
Rank: 11/80
Findings: 2
Award: $2,552.63
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: dipp
Also found by: 0x4non, 0x52, 0xRobocop, 0xc0ffEE, 8olidity, Ch_301, Jeiwan, Junnon, KIntern_NA, Lambda, M4TZ1P, MiloTruck, Nyx, PaludoX0, Ruhum, RustyRabbit, Soosh, TomJ, Trust, arcoun, aviggiano, bardamu, cryptonue, csanuragjain, d3e4, enckrish, exd0tpy, hansfriese, jayphbee, joestakey, ladboy233, minhquanym, minhtrng, nicobevi, obront, polymorphism, rokinot, romand, rotcivegaf, rvierdiiev, saian, serial-coder, trustindistrust, zzykxx
114.8239 USDC - $114.82
https://github.com/code-423n4/2022-10-blur/blob/2fdaa6e13b544c8c11d1c022a575f16c3a72e3bf/contracts/matchingPolicies/StandardPolicyERC1155.sol#L33 https://github.com/code-423n4/2022-10-blur/blob/2fdaa6e13b544c8c11d1c022a575f16c3a72e3bf/contracts/matchingPolicies/StandardPolicyERC1155.sol#L59 https://github.com/code-423n4/2022-10-blur/blob/2fdaa6e13b544c8c11d1c022a575f16c3a72e3bf/contracts/BlurExchange.sol#L145 https://github.com/code-423n4/2022-10-blur/blob/2fdaa6e13b544c8c11d1c022a575f16c3a72e3bf/contracts/BlurExchange.sol#L154-L161 https://github.com/code-423n4/2022-10-blur/blob/2fdaa6e13b544c8c11d1c022a575f16c3a72e3bf/contracts/BlurExchange.sol#L540
The trading amount of an ERC-1155 order is always 1. This will prevent users from trading buying/selling ERC-1155 token with arbitrary amount Also in tests, there is no test case with matching ERC-1155 order with other amounts
https://github.com/code-423n4/2022-10-blur/blob/2fdaa6e13b544c8c11d1c022a575f16c3a72e3bf/contracts/matchingPolicies/StandardPolicyERC1155.sol#L33 https://github.com/code-423n4/2022-10-blur/blob/2fdaa6e13b544c8c11d1c022a575f16c3a72e3bf/contracts/matchingPolicies/StandardPolicyERC1155.sol#L59
These are two LoC that return hardcoded amount of ERC-1155 orders
Manual review
update StandardPolicyERC1155 contract
#0 - blur-io-toad
2022-10-16T23:36:29Z
This was an oversight on my part for not putting this contract as out-of-scope. Our marketplace does not handle ERC1155 yet and so we haven't concluded what the matching critieria for those orders will be. This contract was mainly created to test ERC1155 transfers through the rest of the exchange, but shouldn't be deployed initially. When we are prepared to handle ERC1155 orders we will have to develop a new matching policy that determines the amount from the order parameters. Acknowledging that it's incorrect, but won't be making any changes as the contract won't be deployed.
#1 - GalloDaSballo
2022-10-28T00:03:19Z
🌟 Selected for report: Soosh
Also found by: 0x1f8b, 0x4non, 0xc0ffEE, RaymondFam, Ruhum, RustyRabbit, Trust, arcoun, cccz, d3e4, minhtrng, rotcivegaf, saian, sakshamguruji
The owner could give the address _contract the permission to transfer users funds if they have approved to ExecutionDelegate contract. Says the _contract is a multicall contract, it could pull many users' funds in 1 transaction
Assume that the owner is not malicious, the owner account always has a possibility got hacked and then do malicious actions.
Manual review
Consider using 2 steps process in this function so that users could be aware of the address that could use their funds
#0 - GalloDaSballo
2022-10-27T15:45:17Z
R
#1 - GalloDaSballo
2022-11-15T23:34:13Z
Dup of #235