Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Buyer only receives 1 token when fulfilling ERC1155 order #63

Closed
code423n4 opened this issue Oct 6, 2022 · 1 comment
Closed

Buyer only receives 1 token when fulfilling ERC1155 order #63

code423n4 opened this issue Oct 6, 2022 · 1 comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate This issue or pull request already exists

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-10-blur/blob/main/contracts/matchingPolicies/StandardPolicyERC1155.sol#L33
https://github.com/code-423n4/2022-10-blur/blob/main/contracts/matchingPolicies/StandardPolicyERC1155.sol#L59

Vulnerability details

Impact

The StandardPolicyERC1155 contract always returns 1 for the amount return value. Thus, when fulfilling an order, the exchange only transfers one token from the seller to the buyer, not the value specified in the order.

The user still pays the full price but receives fewer tokens than expected. This is a direct loss of funds and will occur for every ERC1155 order where amount > 1.

Proof of Concept

The user can specify the number of tokens in the order here: https://github.com/code-423n4/2022-10-blur/blob/main/contracts/lib/OrderStructs.sol#L19

The actual number of tokens that is sent when the order is fulfilled is determined using _canMatchOrder(): https://github.com/code-423n4/2022-10-blur/blob/main/contracts/BlurExchange.sol#L145

The function calls the respective policy, in our case StandardPolicyERC1155: https://github.com/code-423n4/2022-10-blur/blob/main/contracts/BlurExchange.sol#L422-L430

The policy's functions always return 1 for the amount return value instead of order.amount: https://github.com/code-423n4/2022-10-blur/blob/main/contracts/matchingPolicies/StandardPolicyERC1155.sol#L33
https://github.com/code-423n4/2022-10-blur/blob/main/contracts/matchingPolicies/StandardPolicyERC1155.sol#L59

Thus, in the end, only one token is sent to the buyer no matter what the value of order.amount was. The value is ignored throughout the whole exchange contract.

Tools Used

none

Recommended Mitigation Steps

Check that the amount value of both orders matches and then return that value instead of 1.

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Oct 6, 2022
code423n4 added a commit that referenced this issue Oct 6, 2022
@GalloDaSballo
Copy link
Collaborator

Dup of #666

@GalloDaSballo GalloDaSballo added the duplicate This issue or pull request already exists label Oct 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

2 participants