ton-blockchain / multisig-contract-v2

Multiowner wallet
54 stars 19 forks source link

Subject: Smart Contract Potential Vulnerabilities - Multisig 2.0 #22

Closed studentofcoding closed 6 months ago

studentofcoding commented 6 months ago

Hi there!

Based on the Smart Contract, I found 1 concern that might lead to unauthorized approvals and bypassing security measures on order.func, which allows an approval to be made without proper authorization checks on this part

https://github.com/ton-blockchain/multisig-contract-v2/blob/107ee13aa4cbabdc9ff0684b738dcd272c4211bc/contracts/order.func#L229C5-L238C6

To address this, we should add authorization checks before allowing approvals to proceed like:

  1. threshold > 0: The threshold should be a positive integer.
  2. threshold <= signers_num: The threshold should be less than or equal to the number of signers.
  3. signers_num >= 1: There should be at least one signer.

which the PoC shows below

// Additional authorization checks
        (int threshold, cell signers, int signers_num, cell proposers) = get_multisig_data();
        throw_unless(error::unauthorized_sign, threshold > 0);
        throw_unless(error::unauthorized_sign, threshold <= signers_num);
        throw_unless(error::unauthorized_sign, signers_num >= 1);

        approve(signer_index, sender_address, query_id);

Best Regards,

Telegram: @mousye_mousye

tolya-yanot commented 6 months ago

chatgpt, ban