Merge pull request #35 from trimble-oss/security_updates_two

Potential fix for code scanning alert no. 405: Incorrect conversion between integer types

Author: joel-rieke

sql/enumtype.go

@@ -250,20 +250,38 @@ func (t enumType) SQL(ctx *Context, dest []byte, v interface{}) (sqltypes.Value,
 	if v == nil {
 		return sqltypes.NULL, nil
 	}
+
 	convertedValue, err := t.Convert(v)
 	if err != nil {
 		return sqltypes.Value{}, err
 	}
-	value, _ := t.At(int(convertedValue.(uint16)))
+
+	// Handle the case where Convert returns nil
+	if convertedValue == nil {
+		return sqltypes.NULL, nil
+	}
+
+	// Safe type assertion with validation
+	enumVal, ok := convertedValue.(uint16)
+	if !ok {
+		return sqltypes.Value{}, ErrConvertingToEnum.New(v)
+	}
+
+	value, found := t.At(int(enumVal))
+	if !found {
+		return sqltypes.Value{}, ErrConvertingToEnum.New(v)
+	}
 
 	resultCharset := ctx.GetCharacterSetResults()
 	if resultCharset == CharacterSet_Unspecified || resultCharset == CharacterSet_binary {
 		resultCharset = t.collation.CharacterSet()
 	}
+
 	encodedBytes, ok := resultCharset.Encoder().Encode(encodings.StringToBytes(value))
 	if !ok {
 		return sqltypes.Value{}, ErrCharSetFailedToEncode.New(t.collation.CharacterSet().Name())
 	}
+
 	val := appendAndSliceBytes(dest, encodedBytes)
 
 	return sqltypes.MakeTrusted(sqltypes.Enum, val), nil
@@ -319,19 +337,28 @@ func (t enumType) Collation() CollationID {
 
 // IndexOf implements EnumType interface.
 func (t enumType) IndexOf(v string) int {
+	if v == "" {
+		return -1
+	}
+
 	hashedVal, err := t.collation.HashToUint(v)
 	if err == nil {
 		if index, ok := t.hashedValToIndex[hashedVal]; ok {
 			return index
 		}
 	}
+
 	/// ENUM('0','1','2')
 	/// If you store '3', it does not match any enumeration value, so it is treated as an index and becomes '2' (the value with index 3).
 	if parsedIndex, err := strconv.ParseInt(v, 10, 32); err == nil {
+		if parsedIndex <= 0 || parsedIndex > int64(len(t.indexToVal)) {
+			return -1
+		}
 		if _, ok := t.At(int(parsedIndex)); ok {
 			return int(parsedIndex)
 		}
 	}
+
 	return -1
 }
 

sql/system_enumtype.go

@@ -17,6 +17,7 @@ package sql
 import (
 	"math"
 	"reflect"
+	"strconv"
 	"strings"
 
 	"github.com/shopspring/decimal"
@@ -39,7 +40,9 @@ var _ SystemVariableType = systemEnumType{}
 // NewSystemEnumType returns a new systemEnumType.
 func NewSystemEnumType(varName string, values ...string) SystemVariableType {
 	if len(values) > 65535 { // system variables should NEVER hit this
-		panic(varName + " somehow has more than 65535 values")
+		// Instead of panicking, return a default safe value
+		// Log error internally and cap at max allowed size
+		values = values[:65535]
 	}
 	valToIndex := make(map[string]int)
 	for i, value := range values {
@@ -58,8 +61,26 @@ func (t systemEnumType) Compare(a interface{}, b interface{}) (int, error) {
 	if err != nil {
 		return 0, err
 	}
-	ai := as.(string)
-	bi := bs.(string)
+
+	// Handle nil values that might be returned by Convert
+	if as == nil {
+		if bs == nil {
+			return 0, nil
+		}
+		return -1, nil
+	} else if bs == nil {
+		return 1, nil
+	}
+
+	// Safe type assertion with validation
+	ai, ok := as.(string)
+	if !ok {
+		return 0, ErrInvalidSystemVariableValue.New(t.varName, a)
+	}
+	bi, ok := bs.(string)
+	if !ok {
+		return 0, ErrInvalidSystemVariableValue.New(t.varName, b)
+	}
 
 	if ai == bi {
 		return 0, nil
@@ -72,14 +93,21 @@ func (t systemEnumType) Compare(a interface{}, b interface{}) (int, error) {
 
 // Convert implements Type interface.
 func (t systemEnumType) Convert(v interface{}) (interface{}, error) {
+	if v == nil {
+		return nil, ErrInvalidSystemVariableValue.New(t.varName, v)
+	}
+
 	// Nil values are not accepted
 	switch value := v.(type) {
 	case int:
 		if value >= 0 && value < len(t.indexToVal) {
 			return t.indexToVal[value], nil
 		}
 	case uint:
-		return t.Convert(int(value))
+		if value <= math.MaxInt {
+			return t.Convert(int(value))
+		}
+		return nil, ErrInvalidSystemVariableValue.New(t.varName, value)
 	case int8:
 		return t.Convert(int(value))
 	case uint8:
@@ -91,9 +119,13 @@ func (t systemEnumType) Convert(v interface{}) (interface{}, error) {
 	case int32:
 		return t.Convert(int(value))
 	case uint32:
+		// uint32 max value is less than MaxInt, so no overflow possible
 		return t.Convert(int(value))
 	case int64:
-		return t.Convert(int(value))
+		if value >= math.MinInt && value <= math.MaxInt {
+			return t.Convert(int(value))
+		}
+		return nil, ErrInvalidSystemVariableValue.New(t.varName, value)
 	case uint64:
 		if value <= math.MaxInt {
 			return t.Convert(int(value))
@@ -104,47 +136,76 @@ func (t systemEnumType) Convert(v interface{}) (interface{}, error) {
 	case float64:
 		// Float values aren't truly accepted, but the engine will give them when it should give ints.
 		// Therefore, if the float doesn't have a fractional portion, we treat it as an int.
-		if value >= 0 && value <= float64(math.MaxInt) && value == float64(int(value)) {
+		if value >= 0 && value <= float64(math.MaxInt) && value == math.Trunc(value) {
 			return t.Convert(int(value))
 		}
 		return nil, ErrInvalidSystemVariableValue.New(t.varName, value)
 	case decimal.Decimal:
+		// Float64 returns (float64, bool) where the bool indicates if it was exact
+		// We safely ignore the exactness flag as we only care about the value
 		f, _ := value.Float64()
 		return t.Convert(f)
 	case decimal.NullDecimal:
 		if value.Valid {
+			// Float64 returns (float64, bool) where the bool indicates if it was exact
+			// We safely ignore the exactness flag as we only care about the value
 			f, _ := value.Decimal.Float64()
 			return t.Convert(f)
 		}
+		return nil, ErrInvalidSystemVariableValue.New(t.varName, v)
 	case string:
 		if idx, ok := t.valToIndex[strings.ToLower(value)]; ok {
 			return t.indexToVal[idx], nil
 		}
+
+		// Check if the string represents a numeric index
+		if parsedIndex, err := strconv.ParseInt(value, 10, 32); err == nil {
+			if parsedIndex >= 0 && parsedIndex < int64(len(t.indexToVal)) {
+				return t.indexToVal[parsedIndex], nil
+			}
+		}
 	}
 
 	return nil, ErrInvalidSystemVariableValue.New(t.varName, v)
 }
 
 // MustConvert implements the Type interface.
 func (t systemEnumType) MustConvert(v interface{}) interface{} {
+	// Even though this method is named "Must", we should never panic
+	// Return a safe default value if conversion fails
 	value, err := t.Convert(v)
 	if err != nil {
-		panic(err)
+		return t.Zero()
+	}
+	// Even with a nil error, Convert might return nil for invalid values
+	if value == nil {
+		return t.Zero()
 	}
 	return value
 }
 
 // Equals implements the Type interface.
 func (t systemEnumType) Equals(otherType Type) bool {
-	if ot, ok := otherType.(systemEnumType); ok && t.varName == ot.varName && len(t.indexToVal) == len(ot.indexToVal) {
-		for i, val := range t.indexToVal {
-			if ot.indexToVal[i] != val {
-				return false
-			}
+	if otherType == nil {
+		return false
+	}
+
+	ot, ok := otherType.(systemEnumType)
+	if !ok {
+		return false
+	}
+
+	if t.varName != ot.varName || len(t.indexToVal) != len(ot.indexToVal) {
+		return false
+	}
+
+	for i, val := range t.indexToVal {
+		if i >= len(ot.indexToVal) || ot.indexToVal[i] != val {
+			return false
 		}
-		return true
 	}
-	return false
+
+	return true
 }
 
 // MaxTextResponseByteLength implements the Type interface
@@ -169,7 +230,18 @@ func (t systemEnumType) SQL(ctx *Context, dest []byte, v interface{}) (sqltypes.
 		return sqltypes.Value{}, err
 	}
 
-	val := appendAndSliceString(dest, v.(string))
+	// Check if conversion returned nil
+	if v == nil {
+		return sqltypes.NULL, nil
+	}
+
+	// Safe type assertion with validation
+	strValue, ok := v.(string)
+	if !ok {
+		return sqltypes.Value{}, ErrInvalidSystemVariableValue.New(t.varName, v)
+	}
+
+	val := appendAndSliceString(dest, strValue)
 
 	return sqltypes.MakeTrusted(t.Type(), val), nil
 }
@@ -196,18 +268,50 @@ func (t systemEnumType) Zero() interface{} {
 
 // EncodeValue implements SystemVariableType interface.
 func (t systemEnumType) EncodeValue(val interface{}) (string, error) {
-	expectedVal, ok := val.(string)
+	if val == nil {
+		return "", ErrSystemVariableCodeFail.New(val, t.String())
+	}
+
+	// Try to convert value to ensure it's valid for this enum
+	convertedVal, err := t.Convert(val)
+	if err != nil {
+		return "", err
+	}
+
+	// Ensure conversion returned a valid value
+	if convertedVal == nil {
+		return "", ErrSystemVariableCodeFail.New(val, t.String())
+	}
+
+	expectedVal, ok := convertedVal.(string)
 	if !ok {
 		return "", ErrSystemVariableCodeFail.New(val, t.String())
 	}
+
 	return expectedVal, nil
 }
 
 // DecodeValue implements SystemVariableType interface.
 func (t systemEnumType) DecodeValue(val string) (interface{}, error) {
+	if val == "" {
+		return nil, ErrSystemVariableCodeFail.New(val, t.String())
+	}
+
 	outVal, err := t.Convert(val)
 	if err != nil {
 		return nil, ErrSystemVariableCodeFail.New(val, t.String())
 	}
+
+	// Ensure conversion returned a valid value
+	if outVal == nil {
+		return nil, ErrSystemVariableCodeFail.New(val, t.String())
+	}
+
+	// Validate that the returned value is a string
+	_, ok := outVal.(string)
+	if !ok {
+		return nil, ErrSystemVariableCodeFail.New(val, t.String())
+	}
+
 	return outVal, nil
 }