tokamak-network/crossTrade

[security vulnerability] A cross trade can be blocked by attackers

Closed this issue · 8 comments

Describe the bug
Attacker can call provideCT with very small _fwAmount. So on L1, the CT is record as a successful trading
=> no more doing: trade, edit, cancel. l2's token will also be blocked on L2

Configuration

  • Severity: HIGH

Impact
A CrossTrade can be blocked

Recommendation
We can check _fwAmount

**Exploit Scenario **

Demo

This seems to overlap with issue29.

As a solution to issue29, I tried to solve it by saving fwAmount to storage. Would it be okay?

Both issue29 and issue31 are related to _fwAmount, but there are 2 scenarios and there are solutions can solve #29 but can not solve #31.

I think saving fwAmount can solve #29 but can not solve this issue. Because L1 does not know the correct _fwAmount on L2 an L1CrossTrade accepts any _fwAmount if there is no edit.

I think if there are solutions, we also need to check carefully to make sure there is no side effect.

@suahnkim This issue and issue #29 seem to be the same. What do you think?
With this solution, I would like to create a separate provide function for when fwAmount must eventually be entered as a hash value and is edited.

yes, I think the hash must contain initial_fwAmount,
during edit, we probably should use a different storage so that the hash does not change based on the updated_fwAmount

thank you @nguyenzung

@nguyenzung the reason why fwAmount was omitted in the hash calculation was that fwAmount can be modified via edit, causing a hash mismatch on L1 and L2, but I think we forgot to account for basic requirement that only requester can set fwAmount and it has to be verified

This issue is related to #29, closing this issue