diff --git a/ql/src/experimental/CWE-681/IncorrectNumericConversion.go b/ql/src/experimental/CWE-681/IncorrectNumericConversion.go index 11c343550ab..518ca38cee0 100644 --- a/ql/src/experimental/CWE-681/IncorrectNumericConversion.go +++ b/ql/src/experimental/CWE-681/IncorrectNumericConversion.go @@ -4,10 +4,17 @@ import ( "strconv" ) -func parseAllocate(wanted string) int32 { +func parseAllocateBad1(wanted string) int32 { parsed, err := strconv.Atoi(wanted) if err != nil { panic(err) } return int32(parsed) } +func parseAllocateBad2(wanted string) int32 { + parsed, err := strconv.ParseInt(wanted, 10, 64) + if err != nil { + panic(err) + } + return int32(parsed) +} diff --git a/ql/src/experimental/CWE-681/IncorrectNumericConversion.qhelp b/ql/src/experimental/CWE-681/IncorrectNumericConversion.qhelp index 77dd3f1cfac..61bf10d9400 100644 --- a/ql/src/experimental/CWE-681/IncorrectNumericConversion.qhelp +++ b/ql/src/experimental/CWE-681/IncorrectNumericConversion.qhelp @@ -5,16 +5,25 @@

-If a numeric value string is parsed using strconv.Atoi into an int, and then that int +If a numeric value string is parsed using strconv.Atoi into an int, and subsequently that int is converted into another type of a lower bit size, the result can produce unexpected values.

+

+This also applie to the results of strconv.ParseFloat, strconv.ParseInt, +and strconv.ParseUint when the specified bit size is higher than the bit size of the +type that number is converted to. +

-If you need to parse numeric values with specific bit sizes, use the functions specific to each -type (strconv.ParseFloat, strconv.ParseInt, strconv.ParseUint) -that also allow to specify the wanted bit size. +If you need to parse numeric values with specific bit sizes, avoid strconv.Atoi, and, instead, +use the functions specific to each type (strconv.ParseFloat, strconv.ParseInt, +strconv.ParseUint) that also allow to specify the wanted bit size. +

+

+When using those functions, be careful to not convert the result to another type with a lower bit size than +the bit size you specified when parsing the number.

If this is not possible, then add upper (and lower) bound checks specific to each type and @@ -24,13 +33,13 @@ bit size (you can find the min and max value for each type in the `math` package

-In the following example, assume that an input string is passed to parseAllocate function, +In the first example, assume that an input string is passed to parseAllocateBad1 function, parsed by strconv.Atoi, and then converted into an int32 type:

The bounds are not checked, so this means that if the provided number is greater than max int32, -the resulting value from the conversion will be different from the provided value. +the resulting value from the conversion will be different from the actual provided value.

To avoid unexpected values, you should either use the other functions provided by the strconv @@ -40,6 +49,24 @@ function.

+ + +

+In the second example, assume that an input string is passed to parseAllocateBad2 function, +parsed by strconv.ParseInt with a bit size set to 64, and then converted into an int32 type: +

+ +

+If the provided number is greater than max int32, the resulting value from the conversion will be +different from the actual provided value. +

+

+To avoid unexpected values, you should specify the correct bit size as in parseAllocateGood3; +or check bounds before making the conversion as in parseAllocateGood4. +

+ +
+
  • mitre.org: CWE-681: Incorrect Conversion between Numeric Types. diff --git a/ql/src/experimental/CWE-681/IncorrectNumericConversionGood.go b/ql/src/experimental/CWE-681/IncorrectNumericConversionGood.go index 003f82cd2a9..29c111cf54e 100644 --- a/ql/src/experimental/CWE-681/IncorrectNumericConversionGood.go +++ b/ql/src/experimental/CWE-681/IncorrectNumericConversionGood.go @@ -30,3 +30,22 @@ func parseAllocateGood2(desired string) int32 { } return int32(parsed) } + +func parseAllocateGood3(wanted string) int32 { + parsed, err := strconv.ParseInt(wanted, 10, 32) + if err != nil { + panic(err) + } + return int32(parsed) +} +func parseAllocateGood4(wanted string) int32 { + parsed, err := strconv.ParseInt(wanted, 10, 64) + if err != nil { + panic(err) + } + // GOOD: check for lower and uppper bounds + if parsed > 0 && parsed <= math.MaxInt32 { + return int32(parsed) + } + return DefaultAllocate +}