hats-finance / Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf

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

Attacker Can Operate Without Gas #99

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

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

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

Description: Description\ Because of IERC1155Receiver(to).onERC1155Received function, recevier can do its own operations. If user calls operateFlowMatrix and sends circle through attacker, attacker can do it without gas.

Attack Scenario\ User call operateFlowMatrix and send circles through attacker.\ operateFlowMatrix()=>_callAcceptanceChecks()=>_acceptanceCheck()=>_doSafeTransferAcceptanceCheck()=>IERC1155Receiver(to).onERC1155Received(operator, from, id, value, data)\ In this function attacker can do operations but he doesn't consume gas. All gas fee will be paid by user. For example, he can call personalmint() In the future, it can cause very dangerous problems like re-entrancy attacks.

Consider to check tx.origin == msg.sender in important public/external functions.

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional)

benjaminbollen commented 2 months ago

ERC1155 standard

NicholeConn1024 commented 2 months ago

In onERC1155Received function, attacker can call personalMint() like "personalMint() reentrancy attack #8". So re-entrancy attack is available and attacker can mint without gas. It's good to consider to check tx.origin == msg.sender.

benjaminbollen commented 2 months ago

it is generally unadvised to rely on tx.origin. But even then, not an issue. Don't send tokens to an attacker contract.

NicholeConn1024 commented 2 months ago

It is not bad to check tx.origin == msg.sender. I know that and I don't recommend to rely on tx.origin. I think user can trust attacker. Attacker doesn't destroy user's account or steal tokens. If user trust attacker, attacker can mint without consuming gas and interacting with contract.

NicholeConn1024 commented 2 months ago

I think it can be low risk at least. Attacker can deceive user and he can mint without interacting with system.

NicholeConn1024 commented 2 months ago

Attacker can mint without interacting with system. This behavior differs from the intended behavior. So I think it can be low risk. And it can be solved by using one modifier(check tx.origin == msg.sender) in personal mint function(and other important functions).