hats-finance / Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf

Circles Protocol contracts
https://aboutcircles.com
GNU Affero General Public License v3.0
0 stars 0 forks source link

result of transferfrom not checked under Migration.sol #58

Open hats-bug-reporter[bot] opened 2 weeks ago

hats-bug-reporter[bot] commented 2 weeks ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xfa88e2d7539668bbc84705cfdda1ebf36cd9870ab3da4c1f0b2f580dacbe53cd Severity: medium

Description: Description\ A call to transferFrom is frequently done without checking the results. For certain ERC20 tokens, if insufficient tokens are present, no revert occurs but a result of "false" is returned. So its important to check this. If you don't you could mint tokens without have received sufficient tokens to do so. So you could loose funds.

// transfer the v1 Circles to this contract to be locked
            circlesV1.transferFrom(msg.sender, address(this), _amounts[i]);

Always check the result of transferFrom.

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional)

benjaminbollen commented 2 weeks ago

Thank you for your report on the unchecked result of transferFrom in Migration.sol. After review, we've determined this is not an issue.

As previously addressed in Issue #30, the transferFrom() function is called on CirclesV1 tokens, which correctly implement the ERC20 standard. This implementation ensures that the function behaves as expected and does not pose any security risks within our system.

We appreciate your thorough examination of our code and your commitment to identifying potential vulnerabilities. Your diligence in reviewing multiple aspects of our contracts contributes to the overall security of the project. Thank you for your continued efforts in this security review.