search

Trust-Machines / BNS-V2

MIT License
10 stars 3 forks source link

Burning STX before checking if the price is met #73

Open BowTiedRadone opened 1 month ago

BowTiedRadone commented 1 month ago

Summary

While fuzzing BNS-V2, @moodmosaic and I noticed that during the namespace-preorder operation, STX is burned without checking if the amount matches the namespace price. We’re not entirely sure if this is an issue or not.

Example

We know that registering a new namespace is a multiple-step operation:

  1. namespace-preorder - The only check for the burned STX amount in this function ensures the amount is positive, but it doesn't verify that it covers the namespace's price. This results in burning user's STX: 
    ;; Confirm that the STX amount to be burned is positive
(asserts! (> stx-to-burn u0) ERR-STX-BURNT-INSUFFICIENT)
;; Execute the token burn operation.
(try! (stx-burn? stx-to-burn tx-sender))
  2. namespace-reveal - If the burned uSTX amount is not enough to cover the price, an error is returned and the namespace is locked for 144 blocks: 
    ;; Verify the burned amount during preorder meets or exceeds the namespace's registration price.
(asserts! (>= (get stx-burned preorder) namespace-price) ERR-STX-BURNT-INSUFFICIENT)

    Conclusion

    While we understand that this method aims to prevent frontrunning by keeping the namespace confidential, we wonder if users could accidentally burn STX, which might be a security concern.

If this behavior is intentional or if we’re misunderstanding something, please feel free to close this issue.

@wileyj @LNow

LNow commented 1 month ago

@BowTiedRadone malicious UI can be used to force user to burn more than it is necessary. I discussed the exact same problem with some folks ~2years ago, as BNS-V1 have same logic. And bare in mind that name-preorder works the same way.

Also if I remember correctly, if you burn not enough to reveal name/namespace then that name/namespace is forever lost (I could be wrong with this one, as I am relying only on my memory with this one.)

IMHO STX should be burned when you reveal name/namespace. And per-orders should be temporal, and if you won't reveal name/namespace withing X blocks (pick X that spans 7+ days), then it can be taken by someone else.

Patotking12 commented 1 week ago

What we want to avoid with the burn happening in the preorder, is people just preordering a lot of namespaces. I will look into a solution and see if something can be implemented