code-423n4 / 2022-03-joyn-findings

4 stars 1 forks source link

Possible problem when mintToken and the user wants to purchase a token #115

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-03-joyn/blob/c9297ccd925ebb2c44dbc6eaa3effd8db5d2368a/core-contracts/contracts/CoreCollection.sol#L139 https://github.com/code-423n4/2022-03-joyn/blob/c9297ccd925ebb2c44dbc6eaa3effd8db5d2368a/core-contracts/contracts/CoreCollection.sol#L162 https://github.com/code-423n4/2022-03-joyn/blob/c9297ccd925ebb2c44dbc6eaa3effd8db5d2368a/core-contracts/contracts/ERC721Payable.sol#L54

Vulnerability details

Impact

A User can mint tokens even if the payment doesn't succeed.

Proof of Concept

the contract ERC721Payable uses erc20 to handle payments. When this contract handles the payment of the user , calls the function transferFrom from the ERC20 payableToken variable and doesn't check if transferFrom succeeded. We don't have the assurance that transferfrom reverts if the operation doesn't succeed, it will depend on each interface.

Tools Used

Recommended Mitigation Steps

Check if transferFrom succeeds before continue.

sofianeOuafir commented 2 years ago

In my opinion, the severity level should be 3 (High Risk) instead of 2 (Med Risk) duplicate of #52