hats-finance / Tapioca-0xe0b920d38a0900af3bab7ff0ca0af554129f54ad

4 stars 4 forks source link

Use of abi.encodePacked() with multiple dynamic arguments #32

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

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

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

Description: Description\ Using abi.encodePacked() with multiple dynamic arguments can result in the same output abi.encodePacked(a,bc) = abi.encodePacked(ab,c). I am not sure if this applies to this case, with the looping but I believe that should an exploiter find a way they can collide values.

Attack Scenario\ Creating two inputs such that abi.encodePacked(a,bc) = abi.encodePacked(ab,c). The location of the LOC is:

tapioca-periph/contracts/tapiocaOmnichainEngine/extension/TapiocaOmnichainEngineHelper.sol#L283

abi.encodePacked(msg_, TapiocaOmnichainEngineCodec.buildYieldBoxPermitAssetMsg(_approvalMsg[i]));

Attachments

  1. Proof of Concept (PoC) File From https://docs.soliditylang.org/en/latest/abi-spec.html

"If you use keccak256(abi.encodePacked(a, b)) and both a and b are dynamic types, it is easy to craft collisions in the hash value by moving parts of a into b and vice-versa. More specifically, abi.encodePacked("a", "bc") == abi.encodePacked("ab", "c"). If you use abi.encodePacked for signatures, authentication or data integrity, make sure to always use the same types and check that at most one of them is dynamic. Unless there is a compelling reason, abi.encode should be preferred."

  1. Revised Code File (Optional)
    • Use abi.encode
    • User a seperator between the two terms to isolate the terms such as abi.encodePacked(msg_, "SEPARATOR", TapiocaOmnichainEngineCodec.buildYieldBoxPermitAssetMsg(_approvalMsg[i]));
0xRektora commented 5 months ago

abi.encodePacked is wanted here. If you looked through the same file, you'd see that there's a function called decodeArrayOfYieldBoxPermitAssetMsg, that uses decodeYieldBoxApprovalAssetMsg, where we slice the data manually .