search

tokamak-network / tokamak-bridge

An interface designed to bridge assets between Titan and Ethereum chains.
https://app.bridge.tokamak.network
6 stars 0 forks source link

[Cross Trade - Contract Bugs]: Validation mismatch for totalAmount/ctAmount parameter between editFee method and requestRegisteredToken method #208

Closed blackcow1987 closed 1 month ago

blackcow1987 commented 2 months ago

What happened?

In requestRegisteredToken method, totalAmount must be greater then ctAmount.

contracts/L2/L2CrossTrade.sol:152

        require(_totalAmount >= _ctAmount, "The totalAmount value must be greater than ctAmount");

However, these restrictions do not exist in the editFee method.

Users can bypass the above restrictions as follows:

  1. Create CrossTrade request using requestRegisteredToken method. (totalAmount >= ctAmount)
  2. Modify ctAmount using editFee method in L1 network. (totalAmount < ctAmoun)

Relevant log output

No response

zzooppii commented 1 month ago

Yes, it is possible. We are aware of that part. Since edit is only possible for the Requester, I made it so that he can change it if he wants to.

This can be easily modified, but since requestNonRegisteredToken supports different tokens, I excluded it from editFee and proceeded.

blackcow1987 commented 1 month ago

@zzooppii My opinion is that if it is possible to bypass the constraint in editFee, We should consider removing this constraint altogether.

zzooppii commented 1 month ago

Thanks for your feedback.

I've already talked about that part, but I'll talk about it again.

zzooppii commented 1 month ago

@blackcow1987

https://github.com/tokamak-network/crossTrade/commit/a2ed6416a419baa8a38ccb6a0638584f933f7397 I added that content to the comment.

Thank you.