EOSIO / eos

An open source smart contract platform
https://developers.eos.io/manuals/eos
MIT License
11.27k stars 3.6k forks source link

Does "asset" type handle overflow/underflow protection by default? #4872

Closed mchxxx closed 5 years ago

mchxxx commented 6 years ago

From asset.hpp (https://github.com/EOSIO/eos/blob/master/contracts/eosiolib/asset.hpp) It seems that the asset type has overflow/underflow protection built-in. For example:

      asset& operator*=( int64_t a ) {
         eosio_assert( a == 0 || (amount * a) / a == amount, "multiplication overflow or underflow" );
         eosio_assert( -max_amount <= amount, "multiplication underflow" );
         eosio_assert( amount <= max_amount,  "multiplication overflow" );
         amount *= a;
         return *this;
      }

However, I did a test, and the result seems to show that there is no overflow/underflow protection. Test:

    uint64_t testa = 10000000000000000000;
    asset test_asset = asset(1, string_to_symbol(4, "EOS")) * testa;

    print("|");
    print(test_asset);
    print("|");
    print(test_asset.amount);
    print("|");

Result:

>> |-844674407370955./*/* EOS|-8446744073709551616|

So, did I do anything wrong or what? If there is no protection by default, what is the best way to prevent overflow/underflow attack?

brianjohnson5972 commented 6 years ago

Code should set: amount *= a; before the asserts for -max_amount < amount < max_amount

taokayan commented 6 years ago

I think we should first do max_amount/abs(amount) to determine the valid range of a, something like:

      asset& operator*=( int64_t a ) {
         if (amount == 0) return *this;
         auto max_a = max_amount / (amount > 0 ? amount : -amount);
         eosio_assert( a >= -max_a && a <= max_a, "multiplication overflow or underflow");
         amount *= a;
         return *this;
      }
cupher commented 6 years ago

Can you let me know an estimated date to fix this issue?

dixia commented 5 years ago

fixed