Swaps security issue: currency/amount info #615
Comments
|
I'll have a PR ready for this tomorrow. |
|
FWIW I don't think this is a bug per se, more like functionality that is lacking. I brought this concern up in #359 , but we went with this oversimplified approach deliberately to get the basic functionality working sooner. Still, I'm thinking I should have created an issue for this right away so that we wouldn't lose track of it, it's certainly critical functionality. |
sangaman
added a commit
that referenced
this issue
Nov 1, 2018
This makes each node involved in a swap determine the swap amounts and currencies (`makerCurrency`, `takerCurrency`, `makerAmount`, and `takerAmount`) independently. If there is any discrepancy in these values, the swap will fail.These values are no longer sent as part of the `SwapRequestPacket`. Previously, only the taker (who initiates the swap) would determine these values and the maker would blindly accept them. It also updates the Swap unit tests to align with the new methods and logic used in the `beginSwap` and `acceptDeal` routines. Closes #615.
ghost
added
the
sangaman
added a commit
that referenced
this issue
Nov 8, 2018
This makes each node involved in a swap determine the swap amounts and currencies (`makerCurrency`, `takerCurrency`, `makerAmount`, and `takerAmount`) independently. If there is any discrepancy in these values, the swap will fail.These values are no longer sent as part of the `SwapRequestPacket`. Previously, only the taker (who initiates the swap) would determine these values and the maker would blindly accept them. It also updates the Swap unit tests to align with the new methods and logic used in the `beginSwap` and `acceptDeal` routines. Closes #615.
sangaman
added a commit
that referenced
this issue
Nov 8, 2018
This makes each node involved in a swap determine the swap amounts and currencies (`makerCurrency`, `takerCurrency`, `makerAmount`, and `takerAmount`) independently. If there is any discrepancy in these values, the swap will fail.These values are no longer sent as part of the `SwapRequestPacket`. Previously, only the taker (who initiates the swap) would determine these values and the maker would blindly accept them. It also updates the Swap unit tests to align with the new methods and logic used in the `beginSwap` and `acceptDeal` routines. Closes #615.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In
OrderBook.handleSwapRequest, the maker verifies his order availability, and accepts the deal. Further validation is applied on LND requests (validateRequests), based on the deal info. But the deal info was never validated on the maker side. It means that the maker is blindly accepting the taker currency/amount calculation in regards to the deal.The text was updated successfully, but these errors were encountered: