fix(exchange): reject int64 overflow in order cost and average price

chain33.fork.toml

@@ -107,6 +107,7 @@ Enable=0
 [fork.sub.exchange]
 Enable=0
 ForkFix1=0
+ForkFixSafeMulOverflow=0
 ForkParamV1 = 0
 ForkParamV2 = 0
 ForkParamV3 = 0

chain33.para.toml

@@ -594,6 +594,7 @@ exchanges = [
 [fork.sub.exchange]
 Enable=0
 ForkFix1=0
+ForkFixSafeMulOverflow=0
 ForkParamV1 = 0
 ForkParamV2 = 0
 ForkParamV3 = 0

plugin/dapp/evm/cmd/ci2/chain33.proxyminer.toml

@@ -615,6 +615,7 @@ Enable=-1
 [fork.sub.exchange]
 Enable=0
 ForkFix1=0
+ForkFixSafeMulOverflow=0
 ForkParamV1 = 0
 ForkParamV2 = 0
 ForkParamV3 = 0

plugin/dapp/exchange/executor/exchange_test.go

@@ -887,7 +887,17 @@ func TestKV(t *testing.T) {
 }
 
 func TestSafeMul(t *testing.T) {
-	t.Log(SafeMul(1e8, 1e7, types.DefaultCoinPrecision))
-	t.Log(SafeMul(1e10, 1e16, types.DefaultCoinPrecision))
-	t.Log(SafeMul(1e7, 1e6, types.DefaultCoinPrecision))
+	v, err := SafeMul(1e8, 1e7, types.DefaultCoinPrecision)
+	t.Log(v, err)
+	assert.Nil(t, err)
+	v, err = SafeMul(1e10, 1e16, types.DefaultCoinPrecision)
+	t.Log(v, err)
+	assert.Nil(t, err)
+	v, err = SafeMul(1e7, 1e6, types.DefaultCoinPrecision)
+	t.Log(v, err)
+	assert.Nil(t, err)
+	// 超出int64时必须返回错误，而不是静默截断
+	if _, err = SafeMul(28999941890838049, 1e16, types.DefaultCoinPrecision); err == nil {
+		t.Fatal("SafeMul should report overflow")
+	}
 }

plugin/dapp/exchange/executor/exchangedb.go

@@ -66,12 +66,35 @@ func (a *Action) OpSwap(op int32) int32 {
 	return et.OpBuy
 }
 
+// isFixSafeMulOverflowFork 判定当前高度是否启用大数乘法溢出修复分叉
+func (a *Action) isFixSafeMulOverflowFork() bool {
+	return a.api.GetConfig().IsDappFork(a.height, et.ExchangeX, et.ForkFixSafeMulOverflow)
+}
+
+// calcActualCostWithFork 计算订单成本；结果溢出时，分叉后返回错误，分叉前保持旧的回绕行为
+func (a *Action) calcActualCostWithFork(op int32, amount, price, coinPrecision int64) (int64, error) {
+	cost, err := CalcActualCost(op, amount, price, coinPrecision)
+	if err != nil && a.isFixSafeMulOverflowFork() {
+		return 0, err
+	}
+	return cost, nil
+}
+
+// caclAVGPriceWithFork 计算平均成交价；结果溢出时，分叉后返回错误，分叉前保持旧的回绕行为
+func (a *Action) caclAVGPriceWithFork(order *et.Order, price, amount int64) (int64, error) {
+	avg, err := caclAVGPrice(order, price, amount)
+	if err != nil && a.isFixSafeMulOverflowFork() {
+		return 0, err
+	}
+	return avg, nil
+}
+
 // CalcActualCost Calculate actual cost
-func CalcActualCost(op int32, amount int64, price, coinPrecision int64) int64 {
+func CalcActualCost(op int32, amount int64, price, coinPrecision int64) (int64, error) {
 	if op == et.OpBuy {
 		return SafeMul(amount, price, coinPrecision)
 	}
-	return amount
+	return amount, nil
 }
 
 // CheckPrice price  1<=price<=1e16
@@ -167,7 +190,11 @@ func (a *Action) LimitOrder(payload *et.LimitOrder, entrustAddr string) (*types.
 	}
 	//Check your account balance first
 	if payload.GetOp() == et.OpBuy {
-		amount := SafeMul(payload.GetAmount(), payload.GetPrice(), cfg.GetCoinPrecision())
+		amount, costErr := SafeMul(payload.GetAmount(), payload.GetPrice(), cfg.GetCoinPrecision())
+		if costErr != nil && a.isFixSafeMulOverflowFork() {
+			elog.Error("limit check right balance overflow", "addr", a.fromaddr, "amount", payload.GetAmount(), "price", payload.GetPrice())
+			return nil, costErr
+		}
 		rightAccount := rightAssetDB.LoadExecAccount(a.fromaddr, a.execaddr)
 		if rightAccount.Balance < amount {
 			elog.Error("limit check right balance", "addr", a.fromaddr, "avail", rightAccount.Balance, "need", amount)
@@ -215,7 +242,10 @@ func (a *Action) RevokeOrder(payload *et.RevokeOrder) (*types.Receipt, error) {
 		if err != nil {
 			return nil, err
 		}
-		amount := CalcActualCost(et.OpBuy, balance, price, cfg.GetCoinPrecision())
+		amount, costErr := a.calcActualCostWithFork(et.OpBuy, balance, price, cfg.GetCoinPrecision())
+		if costErr != nil {
+			return nil, costErr
+		}
 		rightAccount := rightAssetDB.LoadExecAccount(a.fromaddr, a.execaddr)
 		if rightAccount.Frozen < amount {
 			elog.Error("revoke check right frozen", "addr", a.fromaddr, "avail", rightAccount.Frozen, "amount", amount)
@@ -234,7 +264,10 @@ func (a *Action) RevokeOrder(payload *et.RevokeOrder) (*types.Receipt, error) {
 		if err != nil {
 			return nil, err
 		}
-		amount := CalcActualCost(et.OpSell, balance, price, cfg.GetCoinPrecision())
+		amount, costErr := a.calcActualCostWithFork(et.OpSell, balance, price, cfg.GetCoinPrecision())
+		if costErr != nil {
+			return nil, costErr
+		}
 		leftAccount := leftAssetDB.LoadExecAccount(a.fromaddr, a.execaddr)
 		if leftAccount.Frozen < amount {
 			elog.Error("revoke check left frozen", "addr", a.fromaddr, "avail", leftAccount.Frozen, "amount", amount)
@@ -424,7 +457,10 @@ func (a *Action) matchLimitOrder(payload *et.LimitOrder, leftAccountDB, rightAcc
 
 	//Outstanding orders require freezing of the remaining unclosed funds
 	if payload.Op == et.OpBuy {
-		amount := CalcActualCost(et.OpBuy, or.Balance, payload.Price, cfg.GetCoinPrecision())
+		amount, costErr := a.calcActualCostWithFork(et.OpBuy, or.Balance, payload.Price, cfg.GetCoinPrecision())
+		if costErr != nil {
+			return nil, costErr
+		}
 		receipt, err := rightAccountDB.ExecFrozen(a.fromaddr, a.execaddr, amount)
 		if err != nil {
 			elog.Error("LimitOrder.ExecFrozen OpBuy", "addr", a.fromaddr, "amount", amount, "err", err.Error())
@@ -434,7 +470,10 @@ func (a *Action) matchLimitOrder(payload *et.LimitOrder, leftAccountDB, rightAcc
 		kvs = append(kvs, receipt.KV...)
 	}
 	if payload.Op == et.OpSell {
-		amount := CalcActualCost(et.OpSell, or.Balance, payload.Price, cfg.GetCoinPrecision())
+		amount, costErr := a.calcActualCostWithFork(et.OpSell, or.Balance, payload.Price, cfg.GetCoinPrecision())
+		if costErr != nil {
+			return nil, costErr
+		}
 		receipt, err := leftAccountDB.ExecFrozen(a.fromaddr, a.execaddr, amount)
 		if err != nil {
 			elog.Error("LimitOrder.ExecFrozen OpSell", "addr", a.fromaddr, "amount", amount, "err", err.Error())
@@ -468,9 +507,13 @@ func (a *Action) matchModel(leftAccountDB, rightAccountDB *account.DB, payload *
 	cfg := a.api.GetConfig()
 	var receipt *types.Receipt
 	var err error
+	var avgErr error
 	if payload.Op == et.OpSell {
 		//Transfer of frozen assets
-		amount := CalcActualCost(matchorder.GetLimitOrder().Op, matched, matchorder.GetLimitOrder().Price, cfg.GetCoinPrecision())
+		amount, costErr := a.calcActualCostWithFork(matchorder.GetLimitOrder().Op, matched, matchorder.GetLimitOrder().Price, cfg.GetCoinPrecision())
+		if costErr != nil {
+			return nil, nil, costErr
+		}
 		if matchorder.Addr != a.fromaddr {
 			receipt, err = rightAccountDB.ExecTransferFrozen(matchorder.Addr, a.fromaddr, a.execaddr, amount)
 		} else {
@@ -498,7 +541,10 @@ func (a *Action) matchModel(leftAccountDB, rightAccountDB *account.DB, payload *
 		}
 
 		//The settlement of the corresponding assets for the transaction to be concluded
-		amount = CalcActualCost(payload.Op, matched, matchorder.GetLimitOrder().Price, cfg.GetCoinPrecision())
+		amount, costErr = a.calcActualCostWithFork(payload.Op, matched, matchorder.GetLimitOrder().Price, cfg.GetCoinPrecision())
+		if costErr != nil {
+			return nil, nil, costErr
+		}
 		if a.fromaddr != matchorder.Addr {
 			receipt, err = leftAccountDB.ExecTransfer(a.fromaddr, matchorder.Addr, a.execaddr, amount)
 			if err != nil {
@@ -523,12 +569,21 @@ func (a *Action) matchModel(leftAccountDB, rightAccountDB *account.DB, payload *
 			kvs = append(kvs, receipt.KV...)
 		}
 
-		or.AVGPrice = caclAVGPrice(or, matchorder.GetLimitOrder().Price, matched)
+		or.AVGPrice, avgErr = a.caclAVGPriceWithFork(or, matchorder.GetLimitOrder().Price, matched)
+		if avgErr != nil {
+			return nil, nil, avgErr
+		}
 		//Calculate the average transaction price
-		matchorder.AVGPrice = caclAVGPrice(matchorder, matchorder.GetLimitOrder().Price, matched)
+		matchorder.AVGPrice, avgErr = a.caclAVGPriceWithFork(matchorder, matchorder.GetLimitOrder().Price, matched)
+		if avgErr != nil {
+			return nil, nil, avgErr
+		}
 	}
 	if payload.Op == et.OpBuy {
-		amount := CalcActualCost(matchorder.GetLimitOrder().Op, matched, matchorder.GetLimitOrder().Price, cfg.GetCoinPrecision())
+		amount, costErr := a.calcActualCostWithFork(matchorder.GetLimitOrder().Op, matched, matchorder.GetLimitOrder().Price, cfg.GetCoinPrecision())
+		if costErr != nil {
+			return nil, nil, costErr
+		}
 		if a.fromaddr != matchorder.Addr {
 			receipt, err = leftAccountDB.ExecTransferFrozen(matchorder.Addr, a.fromaddr, a.execaddr, amount)
 		} else {
@@ -554,7 +609,10 @@ func (a *Action) matchModel(leftAccountDB, rightAccountDB *account.DB, payload *
 			kvs = append(kvs, receipt.KV...)
 		}
 
-		amount = CalcActualCost(payload.Op, matched, matchorder.GetLimitOrder().Price, cfg.GetCoinPrecision())
+		amount, costErr = a.calcActualCostWithFork(payload.Op, matched, matchorder.GetLimitOrder().Price, cfg.GetCoinPrecision())
+		if costErr != nil {
+			return nil, nil, costErr
+		}
 		if a.fromaddr != matchorder.Addr {
 			receipt, err = rightAccountDB.ExecTransfer(a.fromaddr, matchorder.Addr, a.execaddr, amount)
 			if err != nil {
@@ -578,8 +636,14 @@ func (a *Action) matchModel(leftAccountDB, rightAccountDB *account.DB, payload *
 			kvs = append(kvs, receipt.KV...)
 		}
 
-		or.AVGPrice = caclAVGPrice(or, matchorder.GetLimitOrder().Price, matched)
-		matchorder.AVGPrice = caclAVGPrice(matchorder, matchorder.GetLimitOrder().Price, matched)
+		or.AVGPrice, avgErr = a.caclAVGPriceWithFork(or, matchorder.GetLimitOrder().Price, matched)
+		if avgErr != nil {
+			return nil, nil, avgErr
+		}
+		matchorder.AVGPrice, avgErr = a.caclAVGPriceWithFork(matchorder, matchorder.GetLimitOrder().Price, matched)
+		if avgErr != nil {
+			return nil, nil, avgErr
+		}
 	}
 
 	matchorder.UpdateTime = a.blocktime
@@ -951,20 +1015,27 @@ func queryMarketDepth(marketTable *tab.Table, left, right *et.Asset, op int32, p
 }
 
 // SafeMul Safe multiplication of large numbers, prevent overflow
-func SafeMul(x, y, coinPrecision int64) int64 {
+// 返回截断值与 ErrOverflow；调用方根据分叉决定是拒绝订单还是沿用旧行为
+func SafeMul(x, y, coinPrecision int64) (int64, error) {
 	res := big.NewInt(0).Mul(big.NewInt(x), big.NewInt(y))
 	res = big.NewInt(0).Div(res, big.NewInt(coinPrecision))
-	return res.Int64()
+	if !res.IsInt64() {
+		return res.Int64(), et.ErrOverflow
+	}
+	return res.Int64(), nil
 }
 
 // Calculate the average transaction price
-func caclAVGPrice(order *et.Order, price int64, amount int64) int64 {
+func caclAVGPrice(order *et.Order, price int64, amount int64) (int64, error) {
 	x := big.NewInt(0).Mul(big.NewInt(order.AVGPrice), big.NewInt(order.GetLimitOrder().Amount-order.GetBalance()))
 	y := big.NewInt(0).Mul(big.NewInt(price), big.NewInt(amount))
 	total := big.NewInt(0).Add(x, y)
 	div := big.NewInt(0).Add(big.NewInt(order.GetLimitOrder().Amount-order.GetBalance()), big.NewInt(amount))
 	avg := big.NewInt(0).Div(total, div)
-	return avg.Int64()
+	if !avg.IsInt64() {
+		return avg.Int64(), et.ErrOverflow
+	}
+	return avg.Int64(), nil
 }
 
 // 计Calculation fee

plugin/dapp/exchange/test/exchange_test.go

@@ -270,7 +270,8 @@ func testPlaceLimitOrder(t *testing.T, req *et.LimitOrder, addr string, privkey
 	assert.Nil(t, err)
 	t.Log(tokenAfter)
 
-	cost := executor.CalcActualCost(req.Op, req.Amount, req.Price, types.DefaultCoinPrecision)
+	cost, err := executor.CalcActualCost(req.Op, req.Amount, req.Price, types.DefaultCoinPrecision)
+	assert.Nil(t, err)
 	t.Log(req.Amount, req.Price, cost)
 	// bty/ccny
 	if req.Op == et.OpBuy {
@@ -316,7 +317,8 @@ func testRevokeLimitOrder(t *testing.T, orderID int64, addr string, privkey stri
 	assert.Nil(t, err)
 	t.Log(tokenAfter)
 
-	cost := executor.CalcActualCost(lo.Op, order.Balance, lo.Price, types.DefaultCoinPrecision)
+	cost, err := executor.CalcActualCost(lo.Op, order.Balance, lo.Price, types.DefaultCoinPrecision)
+	assert.Nil(t, err)
 	// bty/ccny
 	if lo.Op == et.OpBuy {
 		// bty

plugin/dapp/exchange/types/errors.go

@@ -18,6 +18,8 @@ var (
 	ErrStatus       = fmt.Errorf("%s", "The status only in  0 , 1, 2!")
 	ErrOrderID      = fmt.Errorf("%s", "Wrong OrderID!")
 
+	ErrOverflow = fmt.Errorf("%s", "The amount calculation overflow int64!")
+
 	ErrCfgFmt     = fmt.Errorf("%s", "ErrCfgFmt")
 	ErrBindAddr   = fmt.Errorf("%s", "The address is not bound")
 	ErrAddrIsBank = fmt.Errorf("%s", "The address cannot be banks")

plugin/dapp/exchange/types/exchange.go

@@ -97,6 +97,9 @@ var (
 	//ForkFix Forks
 	ForkFix1 = "ForkFix1"
 
+	//ForkFixSafeMulOverflow 修复SafeMul/caclAVGPrice大数计算结果超出int64被静默截断的漏洞，溢出时拒绝订单
+	ForkFixSafeMulOverflow = "ForkFixSafeMulOverflow"
+
 	ForkParamV1 = "ForkParamV1"
 	ForkParamV2 = "ForkParamV2"
 	ForkParamV3 = "ForkParamV3"
@@ -141,6 +144,7 @@ func init() {
 func InitFork(cfg *types.Chain33Config) {
 	cfg.RegisterDappFork(ExchangeX, "Enable", 0)
 	cfg.RegisterDappFork(ExchangeX, ForkFix1, 0)
+	cfg.RegisterDappFork(ExchangeX, ForkFixSafeMulOverflow, 0)
 	cfg.RegisterDappFork(ExchangeX, ForkParamV1, 0)
 	cfg.RegisterDappFork(ExchangeX, ForkParamV2, 0)
 	cfg.RegisterDappFork(ExchangeX, ForkParamV3, 0)