search

ton-blockchain / multisig-contract-v2

Multiowner wallet
54 stars 19 forks source link

Bypass order_seqno check? #25

Closed aa22222 closed 6 months ago

aa22222 commented 6 months ago

Not sure if this is a vulnerability per se, but in lines 108 - 115, it is very easy to bypass the order_seqno check. The user can just input order_seqno=MAX_ORDER_SEQNO, and it'll skip the checks even if allow_arbitrary_order_seqno is set to false.

int order_seqno = in_msg_body~load_order_seqno();
if (~ allow_arbitrary_order_seqno) {
  if (order_seqno == MAX_ORDER_SEQNO) {             
    order_seqno = next_order_seqno;
  } else {
    throw_unless(error::invalid_new_order, (order_seqno == next_order_seqno));
  }
  next_order_seqno += 1;
}

Telegram: https://t.me/aadeux Wallet: UQAGRkodDhxfzq76kxqlw2DdO0aejUP_F_U9nfErnJMfWkHF

ProgramCrafter commented 6 months ago

I think that's intended (I'd call such functionality "seqno autofill"). Though, if developers intended this in some other way, next_order_seqno should get checked.

tolya-yanot commented 6 months ago

Yes, it's by design