From 6b28640f170d770dc700e3729d5eb73a52183961 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Wojciech=20Szara=C5=84ski?= <wojciech.szaranski@gmail.com>
Date: Fri, 15 Mar 2024 13:54:48 +0100
Subject: [PATCH] Fix integer overflow
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fixes INCR, DECR, INCRBY, DECRBY to return error when value would
overflow.

Closes: https://github.com/alicebob/miniredis/issues/291

Signed-off-by: Wojciech Szarański <wojciech.szaranski@gmail.com>
---
 cmd_string_test.go         | 46 ++++++++++++++++++++++++++++++++++++++
 db.go                      | 18 +++++++++++++++
 direct.go                  |  3 +++
 integration/string_test.go | 11 +++++++++
 redis.go                   |  1 +
 5 files changed, 79 insertions(+)

diff --git a/cmd_string_test.go b/cmd_string_test.go
index 2121f2a5..636d8810 100644
--- a/cmd_string_test.go
+++ b/cmd_string_test.go
@@ -479,6 +479,15 @@ func TestIncr(t *testing.T) {
 		assert(t, err != nil, "do s.Incr error")
 	}
 
+	// Overflow
+	{
+		s.Set("overflow", "9223372036854775807")
+		mustDo(t, c,
+			"INCR", "overflow",
+			proto.Error(msgIntOverflow),
+		)
+	}
+
 	// Wrong usage
 	{
 		mustDo(t, c,
@@ -543,6 +552,20 @@ func TestIncrBy(t *testing.T) {
 		proto.Error(msgInvalidInt),
 	)
 
+	// Overflow
+	{
+		s.Set("overflow", "10")
+		mustDo(t, c,
+			"INCRBY", "overflow", "9223372036854775807",
+			proto.Error(msgIntOverflow),
+		)
+		s.Set("overflow", "-10")
+		mustDo(t, c,
+			"INCRBY", "overflow", "-9223372036854775807",
+			proto.Error(msgIntOverflow),
+		)
+	}
+
 	// Wrong usage
 	{
 		mustDo(t, c,
@@ -684,6 +707,20 @@ func TestDecrBy(t *testing.T) {
 		proto.Error(msgInvalidInt),
 	)
 
+	// Overflow
+	{
+		s.Set("overflow", "10")
+		mustDo(t, c,
+			"DECRBY", "overflow", "-9223372036854775807",
+			proto.Error(msgIntOverflow),
+		)
+		s.Set("overflow", "-10")
+		mustDo(t, c,
+			"DECRBY", "overflow", "9223372036854775807",
+			proto.Error(msgIntOverflow),
+		)
+	}
+
 	// Wrong usage
 	{
 		mustDo(t, c,
@@ -754,6 +791,15 @@ func TestDecr(t *testing.T) {
 		)
 	}
 
+	// Overflow
+	{
+		s.Set("overflow", "-9223372036854775808")
+		mustDo(t, c,
+			"DECR", "overflow",
+			proto.Error(msgIntOverflow),
+		)
+	}
+
 	// Direct one works
 	{
 		s.Set("aap", "400")
diff --git a/db.go b/db.go
index 9227866c..f77a88d3 100644
--- a/db.go
+++ b/db.go
@@ -9,6 +9,13 @@ import (
 	"time"
 )
 
+const (
+	intSize = 32 << (^uint(0) >> 63) // 32 or 64
+
+	maxInt = 1<<(intSize-1) - 1  // [math.MaxInt] was added in go 1.17
+	minInt = -1 << (intSize - 1) // [math.MinInt] was added in go 1.17
+)
+
 var (
 	errInvalidEntryID = errors.New("stream ID is invalid")
 )
@@ -180,6 +187,17 @@ func (db *RedisDB) stringIncr(k string, delta int) (int, error) {
 			return 0, ErrIntValueError
 		}
 	}
+
+	if delta > 0 {
+		if maxInt-delta < v {
+			return 0, ErrIntValueOverflowError
+		}
+	} else {
+		if minInt-delta > v {
+			return 0, ErrIntValueOverflowError
+		}
+	}
+
 	v += delta
 	db.stringSet(k, strconv.Itoa(v))
 	return v, nil
diff --git a/direct.go b/direct.go
index 1834aa5a..31505eb4 100644
--- a/direct.go
+++ b/direct.go
@@ -21,6 +21,9 @@ var (
 	// ErrIntValueError can returned by INCRBY
 	ErrIntValueError = errors.New(msgInvalidInt)
 
+	// ErrIntValueOverflowError can be returned by INCR, DECR, INCRBY, DECRBY
+	ErrIntValueOverflowError = errors.New(msgIntOverflow)
+
 	// ErrFloatValueError can returned by INCRBYFLOAT
 	ErrFloatValueError = errors.New(msgInvalidFloat)
 )
diff --git a/integration/string_test.go b/integration/string_test.go
index c1877e54..60828404 100644
--- a/integration/string_test.go
+++ b/integration/string_test.go
@@ -410,6 +410,17 @@ func TestIncrAndFriends(t *testing.T) {
 		c.Do("INCRBYFLOAT", "zero", "12.3")
 		c.Do("INCRBYFLOAT", "zero", "-13.1")
 
+		// Overflow
+		c.Do("SET", "overflow-up", "9223372036854775807")
+		c.Error("increment or decrement would overflow", "INCR", "overflow-up")
+		c.Error("increment or decrement would overflow", "INCRBY", "overflow-up", "1")
+		c.Error("increment or decrement would overflow", "DECRBY", "overflow-up", "-1")
+
+		c.Do("SET", "overflow-down", "-9223372036854775808")
+		c.Error("increment or decrement would overflow", "DECR", "overflow-down")
+		c.Error("increment or decrement would overflow", "INCRBY", "overflow-down", "-1")
+		c.Error("increment or decrement would overflow", "DECRBY", "overflow-down", "1")
+
 		// E
 		c.Do("INCRBYFLOAT", "one", "12e12")
 		// c.Do("INCRBYFLOAT", "one", "12e34") // FIXME
diff --git a/redis.go b/redis.go
index 5a21fe7b..f70728e7 100644
--- a/redis.go
+++ b/redis.go
@@ -16,6 +16,7 @@ const (
 	msgWrongType            = "WRONGTYPE Operation against a key holding the wrong kind of value"
 	msgNotValidHllValue     = "WRONGTYPE Key is not a valid HyperLogLog string value."
 	msgInvalidInt           = "ERR value is not an integer or out of range"
+	msgIntOverflow          = "ERR increment or decrement would overflow"
 	msgInvalidFloat         = "ERR value is not a valid float"
 	msgInvalidMinMax        = "ERR min or max is not a float"
 	msgInvalidRangeItem     = "ERR min or max not valid string range item"