diff --git a/ql/src/experimental/CWE-681/IncorrectNumericConversion.go b/ql/src/experimental/CWE-681/IncorrectNumericConversion.go new file mode 100644 index 00000000000..518ca38cee0 --- /dev/null +++ b/ql/src/experimental/CWE-681/IncorrectNumericConversion.go @@ -0,0 +1,20 @@ +package main + +import ( + "strconv" +) + +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 new file mode 100644 index 00000000000..f978858ed89 --- /dev/null +++ b/ql/src/experimental/CWE-681/IncorrectNumericConversion.qhelp @@ -0,0 +1,65 @@ + + + +

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

+

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

+
+ +

+ 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 smaller 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 + bit size (you can find the minimum and maximum value for each type in the `math` package). +

+
+ +

+ 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 the maximum value of type int32, + 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 + package to parse the specific types and bit sizes as shown in the + parseAllocateGood2 function; or check bounds as in the parseAllocateGood1 + 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 the maximum value of type 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. +

+ +
+
diff --git a/ql/src/experimental/CWE-681/IncorrectNumericConversion.ql b/ql/src/experimental/CWE-681/IncorrectNumericConversion.ql new file mode 100644 index 00000000000..def77b696e9 --- /dev/null +++ b/ql/src/experimental/CWE-681/IncorrectNumericConversion.ql @@ -0,0 +1,266 @@ +/** + * @name Incorrect conversion between numeric types + * @description Converting the result of strconv.Atoi (and other parsers from strconv package) + * to numeric types of smaller bit size can produce unexpected values. + * @kind path-problem + * @problem.severity warning + * @id go/incorrect-numeric-conversion + * @tags security + * external/cwe/cwe-190 + * external/cwe/cwe-681 + */ + +import go +import DataFlow::PathGraph + +/** A function that parses integers. */ +class Atoi extends Function { + Atoi() { this.hasQualifiedName("strconv", "Atoi") } +} + +/** A function that parses floating-point numbers. */ +class ParseFloat extends Function { + ParseFloat() { this.hasQualifiedName("strconv", "ParseFloat") } +} + +/** A function that parses integers with a specifiable bitSize. */ +class ParseInt extends Function { + ParseInt() { this.hasQualifiedName("strconv", "ParseInt") } +} + +/** A function that parses unsigned integers with a specifiable bitSize. */ +class ParseUint extends Function { + ParseUint() { this.hasQualifiedName("strconv", "ParseUint") } +} + +/** Provides a class for modeling calls to number-parsing functions. */ +module ParserCall { + /** + * A data-flow call node that parses a number. + */ + abstract class Range extends DataFlow::CallNode { + /** Gets the bit size of the result number. */ + abstract int getTargetBitSize(); + + /** Gets the name of the parser function. */ + abstract string getParserName(); + } +} + +class ParserCall extends DataFlow::CallNode { + ParserCall::Range self; + + ParserCall() { this = self } + + int getTargetBitSize() { result = self.getTargetBitSize() } + + string getParserName() { result = self.getParserName() } +} + +class AtoiCall extends DataFlow::CallNode, ParserCall::Range { + AtoiCall() { exists(Atoi atoi | this = atoi.getACall()) } + + override int getTargetBitSize() { result = 0 } + + override string getParserName() { result = "strconv.Atoi" } +} + +class ParseIntCall extends DataFlow::CallNode, ParserCall::Range { + ParseIntCall() { exists(ParseInt parseInt | this = parseInt.getACall()) } + + override int getTargetBitSize() { result = this.getArgument(2).getIntValue() } + + override string getParserName() { result = "strconv.ParseInt" } +} + +class ParseUintCall extends DataFlow::CallNode, ParserCall::Range { + ParseUintCall() { exists(ParseUint parseUint | this = parseUint.getACall()) } + + override int getTargetBitSize() { result = this.getArgument(2).getIntValue() } + + override string getParserName() { result = "strconv.ParseUint" } +} + +class ParseFloatCall extends DataFlow::CallNode, ParserCall::Range { + ParseFloatCall() { exists(ParseFloat parseFloat | this = parseFloat.getACall()) } + + override int getTargetBitSize() { result = this.getArgument(1).getIntValue() } + + override string getParserName() { result = "strconv.ParseFloat" } +} + +class NumericConversionExpr extends ConversionExpr { + string fullTypeName; + int bitSize; + + NumericConversionExpr() { + exists(NumericType conv | + conv = getTypeExpr().getType().getUnderlyingType() and + fullTypeName = conv.getName() and + bitSize = conv.getSize() + ) + } + + string getFullTypeName() { result = fullTypeName } + + int getBitSize() { result = bitSize } +} + +/** + * An `if` statement with the condition being either a relational comparison, + * or one or more `&&`. + */ +class IfRelationalComparison extends IfStmt { + IfRelationalComparison() { + this.getCond() instanceof RelationalComparisonExpr or this.getCond() instanceof LandExpr + } + + RelationalComparisonExpr getComparison() { result = this.getCond().(RelationalComparisonExpr) } + + LandExpr getLandExpr() { result = this.getCond().(LandExpr) } +} + +/** + * Flow of result of parsing a 64 bit number, to conversion to lower bit numbers. + */ +class Lt64BitFlowConfig extends TaintTracking::Configuration, DataFlow::Configuration { + Lt64BitFlowConfig() { this = "Lt64BitFlowConfig" } + + override predicate isSource(DataFlow::Node source) { + exists(ParserCall call | call.getTargetBitSize() = [0, 64] | source = call) + } + + override predicate isSink(DataFlow::Node sink) { + exists(NumericConversionExpr conv | conv.getBitSize() = [32, 16, 8] | sink.asExpr() = conv) + } + + override predicate isSanitizerIn(DataFlow::Node node) { isSanitizedInsideAnIfBoundCheck(node) } +} + +/** + * Flow of result of parsing a 32 bit number, to conversion to lower bit numbers. + */ +class Lt32BitFlowConfig extends TaintTracking::Configuration, DataFlow::Configuration { + Lt32BitFlowConfig() { this = "Lt32BitFlowConfig" } + + override predicate isSource(DataFlow::Node source) { + // NOTE: target bit size 0 is already addressed in Lt64BitFlowConfig. + exists(ParserCall call | call.getTargetBitSize() = [/*0,*/ 32] | source = call) + } + + override predicate isSink(DataFlow::Node sink) { + exists(NumericConversionExpr conv | conv.getBitSize() = [16, 8] | sink.asExpr() = conv) + } + + override predicate isSanitizerIn(DataFlow::Node node) { isSanitizedInsideAnIfBoundCheck(node) } +} + +/** + * Flow of result of parsing a 16 bit number, to conversion to lower bit numbers. + */ +class Lt16BitFlowConfig extends TaintTracking::Configuration, DataFlow::Configuration { + Lt16BitFlowConfig() { this = "Lt16BitFlowConfig" } + + override predicate isSource(DataFlow::Node source) { + exists(ParserCall call | call.getTargetBitSize() = 16 | source = call) + } + + override predicate isSink(DataFlow::Node sink) { + exists(NumericConversionExpr conv | conv.getBitSize() = 8 | sink.asExpr() = conv) + } + + override predicate isSanitizerIn(DataFlow::Node node) { isSanitizedInsideAnIfBoundCheck(node) } +} + +/** + * Check if the node is a numeric conversion inside an `if` body, where + * the `if` condition contains an upper bound check on the conversion operand. + */ +predicate isSanitizedInsideAnIfBoundCheck(DataFlow::Node node) { + exists(IfRelationalComparison comp, NumericConversionExpr conv | + conv = node.asExpr().(NumericConversionExpr) and + conv.getBitSize() = [8, 16, 32] and + comp.getThen().getAChild*() = conv and + ( + // If the conversion is inside an `if` block that compares the source as + // `source > 0` or `source >= 0`, then that sanitizes conversion of int to int32; + conv.getFullTypeName() = "int32" and + comp.getComparison().getLesserOperand().getNumericValue() = 0 and + comp.getComparison().getGreaterOperand().getGlobalValueNumber() = + conv.getOperand().getGlobalValueNumber() + or + comparisonGreaterOperandValueIsEqual("int8", comp, conv, getMaxInt8()) + or + comparisonGreaterOperandValueIsEqual("int16", comp, conv, getMaxInt16()) + or + comparisonGreaterOperandValueIsEqual("int32", comp, conv, getMaxInt32()) + or + comparisonGreaterOperandValueIsEqual("uint8", comp, conv, getMaxUint8()) + or + comparisonGreaterOperandValueIsEqual("uint16", comp, conv, getMaxUint16()) + ) + ) +} + +int getMaxInt8() { result = 2.pow(7) - 1 } + +int getMaxInt16() { result = 2.pow(15) - 1 } + +int getMaxInt32() { result = 2.pow(31) - 1 } + +int getMaxUint8() { result = 2.pow(8) - 1 } + +int getMaxUint16() { result = 2.pow(16) - 1 } + +/** + * The `if` relational comparison (which can also be inside a `LandExpr`) stating that + * the greater operand is equal to `value`, and the lesses operand is the conversion operand. + */ +predicate comparisonGreaterOperandValueIsEqual( + string typeName, IfRelationalComparison ifExpr, NumericConversionExpr conv, int value +) { + conv.getFullTypeName() = typeName and + ( + // exclude cases like: if parsed < math.MaxInt8 {return int8(parsed)} + exists(RelationalComparisonExpr comp | comp = ifExpr.getComparison() | + // greater operand is equal to value: + comp.getGreaterOperand().getNumericValue() = value and + // and lesser is the conversion operand: + comp.getLesserOperand().getGlobalValueNumber() = conv.getOperand().getGlobalValueNumber() + ) + or + // exclude cases like: if err == nil && parsed < math.MaxInt8 {return int8(parsed)} + exists(RelationalComparisonExpr andExpr | + andExpr = ifExpr.getLandExpr().getAnOperand().(RelationalComparisonExpr) + | + // greater operand is equal to value: + andExpr.getGreaterOperand().getNumericValue() = value and + // and lesser is the conversion operand: + andExpr.getLesserOperand().getGlobalValueNumber() = conv.getOperand().getGlobalValueNumber() + ) + ) +} + +string formatBitSize(ParserCall call) { + call.getTargetBitSize() = 0 and result = "(arch-dependent)" + or + call.getTargetBitSize() > 0 and result = call.getTargetBitSize().toString() +} + +from DataFlow::PathNode source, DataFlow::PathNode sink +where + ( + exists(Lt64BitFlowConfig cfg | cfg.hasFlowPath(source, sink)) + or + exists(Lt32BitFlowConfig cfg | cfg.hasFlowPath(source, sink)) + or + exists(Lt16BitFlowConfig cfg | cfg.hasFlowPath(source, sink)) + ) and + // Exclude results in test files: + exists(File fl | fl = sink.getNode().asExpr().(NumericConversionExpr).getFile() | + not fl instanceof TestFile + ) +select source.getNode(), source, sink, + "Incorrect conversion of a " + formatBitSize(source.getNode().(ParserCall)) + "-bit number from " + + source.getNode().(ParserCall).getParserName() + " result to a lower bit size type " + + sink.getNode().asExpr().(NumericConversionExpr).getFullTypeName() diff --git a/ql/src/experimental/CWE-681/IncorrectNumericConversionGood.go b/ql/src/experimental/CWE-681/IncorrectNumericConversionGood.go new file mode 100644 index 00000000000..29c111cf54e --- /dev/null +++ b/ql/src/experimental/CWE-681/IncorrectNumericConversionGood.go @@ -0,0 +1,51 @@ +package main + +import ( + "math" + "strconv" +) + +func main() { + +} + +const DefaultAllocate int32 = 256 + +func parseAllocateGood1(desired string) int32 { + parsed, err := strconv.Atoi(desired) + if err != nil { + return DefaultAllocate + } + // GOOD: check for lower and uppper bounds + if parsed > 0 && parsed <= math.MaxInt32 { + return int32(parsed) + } + return DefaultAllocate +} +func parseAllocateGood2(desired string) int32 { + // GOOD: parse specifying the bit size + parsed, err := strconv.ParseInt(desired, 10, 32) + if err != nil { + return DefaultAllocate + } + 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 +} diff --git a/ql/test/experimental/CWE-681/IncorrectNumericConversion.expected b/ql/test/experimental/CWE-681/IncorrectNumericConversion.expected new file mode 100644 index 00000000000..64aa194012e --- /dev/null +++ b/ql/test/experimental/CWE-681/IncorrectNumericConversion.expected @@ -0,0 +1,83 @@ +edges +| IncorrectNumericConversion.go:26:14:26:28 | call to Atoi : tuple type | IncorrectNumericConversion.go:35:41:35:50 | type conversion | +| IncorrectNumericConversion.go:53:18:53:47 | call to ParseFloat : tuple type | IncorrectNumericConversion.go:57:7:57:19 | type conversion | +| IncorrectNumericConversion.go:60:18:60:47 | call to ParseFloat : tuple type | IncorrectNumericConversion.go:64:7:64:19 | type conversion | +| IncorrectNumericConversion.go:69:18:69:49 | call to ParseInt : tuple type | IncorrectNumericConversion.go:73:7:73:18 | type conversion | +| IncorrectNumericConversion.go:76:18:76:49 | call to ParseInt : tuple type | IncorrectNumericConversion.go:80:7:80:19 | type conversion | +| IncorrectNumericConversion.go:83:18:83:49 | call to ParseInt : tuple type | IncorrectNumericConversion.go:87:7:87:19 | type conversion | +| IncorrectNumericConversion.go:90:18:90:48 | call to ParseInt : tuple type | IncorrectNumericConversion.go:94:7:94:19 | type conversion | +| IncorrectNumericConversion.go:99:18:99:50 | call to ParseUint : tuple type | IncorrectNumericConversion.go:103:7:103:18 | type conversion | +| IncorrectNumericConversion.go:106:18:106:50 | call to ParseUint : tuple type | IncorrectNumericConversion.go:110:7:110:19 | type conversion | +| IncorrectNumericConversion.go:113:18:113:50 | call to ParseUint : tuple type | IncorrectNumericConversion.go:117:7:117:19 | type conversion | +| IncorrectNumericConversion.go:120:18:120:49 | call to ParseUint : tuple type | IncorrectNumericConversion.go:124:7:124:19 | type conversion | +| IncorrectNumericConversion.go:208:18:208:36 | call to Atoi : tuple type | IncorrectNumericConversion.go:212:7:212:18 | type conversion | +| IncorrectNumericConversion.go:215:18:215:36 | call to Atoi : tuple type | IncorrectNumericConversion.go:219:7:219:19 | type conversion | +| IncorrectNumericConversion.go:222:18:222:36 | call to Atoi : tuple type | IncorrectNumericConversion.go:226:7:226:19 | type conversion | +| IncorrectNumericConversion.go:229:18:229:36 | call to Atoi : tuple type | IncorrectNumericConversion.go:233:7:233:19 | type conversion | +| IncorrectNumericConversion.go:236:18:236:36 | call to Atoi : tuple type | IncorrectNumericConversion.go:240:7:240:20 | type conversion | +| IncorrectNumericConversion.go:243:18:243:36 | call to Atoi : tuple type | IncorrectNumericConversion.go:247:7:247:20 | type conversion | +| IncorrectNumericConversion.go:250:18:250:36 | call to Atoi : tuple type | IncorrectNumericConversion.go:254:7:254:21 | type conversion | +| IncorrectNumericConversion.go:257:18:257:36 | call to Atoi : tuple type | IncorrectNumericConversion.go:262:7:262:18 | type conversion | +| IncorrectNumericConversion.go:266:18:266:36 | call to Atoi : tuple type | IncorrectNumericConversion.go:270:7:270:23 | type conversion | +nodes +| IncorrectNumericConversion.go:26:14:26:28 | call to Atoi : tuple type | semmle.label | call to Atoi : tuple type | +| IncorrectNumericConversion.go:35:41:35:50 | type conversion | semmle.label | type conversion | +| IncorrectNumericConversion.go:53:18:53:47 | call to ParseFloat : tuple type | semmle.label | call to ParseFloat : tuple type | +| IncorrectNumericConversion.go:57:7:57:19 | type conversion | semmle.label | type conversion | +| IncorrectNumericConversion.go:60:18:60:47 | call to ParseFloat : tuple type | semmle.label | call to ParseFloat : tuple type | +| IncorrectNumericConversion.go:64:7:64:19 | type conversion | semmle.label | type conversion | +| IncorrectNumericConversion.go:69:18:69:49 | call to ParseInt : tuple type | semmle.label | call to ParseInt : tuple type | +| IncorrectNumericConversion.go:73:7:73:18 | type conversion | semmle.label | type conversion | +| IncorrectNumericConversion.go:76:18:76:49 | call to ParseInt : tuple type | semmle.label | call to ParseInt : tuple type | +| IncorrectNumericConversion.go:80:7:80:19 | type conversion | semmle.label | type conversion | +| IncorrectNumericConversion.go:83:18:83:49 | call to ParseInt : tuple type | semmle.label | call to ParseInt : tuple type | +| IncorrectNumericConversion.go:87:7:87:19 | type conversion | semmle.label | type conversion | +| IncorrectNumericConversion.go:90:18:90:48 | call to ParseInt : tuple type | semmle.label | call to ParseInt : tuple type | +| IncorrectNumericConversion.go:94:7:94:19 | type conversion | semmle.label | type conversion | +| IncorrectNumericConversion.go:99:18:99:50 | call to ParseUint : tuple type | semmle.label | call to ParseUint : tuple type | +| IncorrectNumericConversion.go:103:7:103:18 | type conversion | semmle.label | type conversion | +| IncorrectNumericConversion.go:106:18:106:50 | call to ParseUint : tuple type | semmle.label | call to ParseUint : tuple type | +| IncorrectNumericConversion.go:110:7:110:19 | type conversion | semmle.label | type conversion | +| IncorrectNumericConversion.go:113:18:113:50 | call to ParseUint : tuple type | semmle.label | call to ParseUint : tuple type | +| IncorrectNumericConversion.go:117:7:117:19 | type conversion | semmle.label | type conversion | +| IncorrectNumericConversion.go:120:18:120:49 | call to ParseUint : tuple type | semmle.label | call to ParseUint : tuple type | +| IncorrectNumericConversion.go:124:7:124:19 | type conversion | semmle.label | type conversion | +| IncorrectNumericConversion.go:208:18:208:36 | call to Atoi : tuple type | semmle.label | call to Atoi : tuple type | +| IncorrectNumericConversion.go:212:7:212:18 | type conversion | semmle.label | type conversion | +| IncorrectNumericConversion.go:215:18:215:36 | call to Atoi : tuple type | semmle.label | call to Atoi : tuple type | +| IncorrectNumericConversion.go:219:7:219:19 | type conversion | semmle.label | type conversion | +| IncorrectNumericConversion.go:222:18:222:36 | call to Atoi : tuple type | semmle.label | call to Atoi : tuple type | +| IncorrectNumericConversion.go:226:7:226:19 | type conversion | semmle.label | type conversion | +| IncorrectNumericConversion.go:229:18:229:36 | call to Atoi : tuple type | semmle.label | call to Atoi : tuple type | +| IncorrectNumericConversion.go:233:7:233:19 | type conversion | semmle.label | type conversion | +| IncorrectNumericConversion.go:236:18:236:36 | call to Atoi : tuple type | semmle.label | call to Atoi : tuple type | +| IncorrectNumericConversion.go:240:7:240:20 | type conversion | semmle.label | type conversion | +| IncorrectNumericConversion.go:243:18:243:36 | call to Atoi : tuple type | semmle.label | call to Atoi : tuple type | +| IncorrectNumericConversion.go:247:7:247:20 | type conversion | semmle.label | type conversion | +| IncorrectNumericConversion.go:250:18:250:36 | call to Atoi : tuple type | semmle.label | call to Atoi : tuple type | +| IncorrectNumericConversion.go:254:7:254:21 | type conversion | semmle.label | type conversion | +| IncorrectNumericConversion.go:257:18:257:36 | call to Atoi : tuple type | semmle.label | call to Atoi : tuple type | +| IncorrectNumericConversion.go:262:7:262:18 | type conversion | semmle.label | type conversion | +| IncorrectNumericConversion.go:266:18:266:36 | call to Atoi : tuple type | semmle.label | call to Atoi : tuple type | +| IncorrectNumericConversion.go:270:7:270:23 | type conversion | semmle.label | type conversion | +#select +| IncorrectNumericConversion.go:26:14:26:28 | call to Atoi | IncorrectNumericConversion.go:26:14:26:28 | call to Atoi : tuple type | IncorrectNumericConversion.go:35:41:35:50 | type conversion | Incorrect conversion of a (arch-dependent)-bit number from strconv.Atoi result to a lower bit size type int32 | +| IncorrectNumericConversion.go:53:18:53:47 | call to ParseFloat | IncorrectNumericConversion.go:53:18:53:47 | call to ParseFloat : tuple type | IncorrectNumericConversion.go:57:7:57:19 | type conversion | Incorrect conversion of a 32-bit number from strconv.ParseFloat result to a lower bit size type int16 | +| IncorrectNumericConversion.go:60:18:60:47 | call to ParseFloat | IncorrectNumericConversion.go:60:18:60:47 | call to ParseFloat : tuple type | IncorrectNumericConversion.go:64:7:64:19 | type conversion | Incorrect conversion of a 64-bit number from strconv.ParseFloat result to a lower bit size type int32 | +| IncorrectNumericConversion.go:69:18:69:49 | call to ParseInt | IncorrectNumericConversion.go:69:18:69:49 | call to ParseInt : tuple type | IncorrectNumericConversion.go:73:7:73:18 | type conversion | Incorrect conversion of a 16-bit number from strconv.ParseInt result to a lower bit size type int8 | +| IncorrectNumericConversion.go:76:18:76:49 | call to ParseInt | IncorrectNumericConversion.go:76:18:76:49 | call to ParseInt : tuple type | IncorrectNumericConversion.go:80:7:80:19 | type conversion | Incorrect conversion of a 32-bit number from strconv.ParseInt result to a lower bit size type int16 | +| IncorrectNumericConversion.go:83:18:83:49 | call to ParseInt | IncorrectNumericConversion.go:83:18:83:49 | call to ParseInt : tuple type | IncorrectNumericConversion.go:87:7:87:19 | type conversion | Incorrect conversion of a 64-bit number from strconv.ParseInt result to a lower bit size type int32 | +| IncorrectNumericConversion.go:90:18:90:48 | call to ParseInt | IncorrectNumericConversion.go:90:18:90:48 | call to ParseInt : tuple type | IncorrectNumericConversion.go:94:7:94:19 | type conversion | Incorrect conversion of a (arch-dependent)-bit number from strconv.ParseInt result to a lower bit size type int32 | +| IncorrectNumericConversion.go:99:18:99:50 | call to ParseUint | IncorrectNumericConversion.go:99:18:99:50 | call to ParseUint : tuple type | IncorrectNumericConversion.go:103:7:103:18 | type conversion | Incorrect conversion of a 16-bit number from strconv.ParseUint result to a lower bit size type int8 | +| IncorrectNumericConversion.go:106:18:106:50 | call to ParseUint | IncorrectNumericConversion.go:106:18:106:50 | call to ParseUint : tuple type | IncorrectNumericConversion.go:110:7:110:19 | type conversion | Incorrect conversion of a 32-bit number from strconv.ParseUint result to a lower bit size type int16 | +| IncorrectNumericConversion.go:113:18:113:50 | call to ParseUint | IncorrectNumericConversion.go:113:18:113:50 | call to ParseUint : tuple type | IncorrectNumericConversion.go:117:7:117:19 | type conversion | Incorrect conversion of a 64-bit number from strconv.ParseUint result to a lower bit size type int32 | +| IncorrectNumericConversion.go:120:18:120:49 | call to ParseUint | IncorrectNumericConversion.go:120:18:120:49 | call to ParseUint : tuple type | IncorrectNumericConversion.go:124:7:124:19 | type conversion | Incorrect conversion of a (arch-dependent)-bit number from strconv.ParseUint result to a lower bit size type int32 | +| IncorrectNumericConversion.go:208:18:208:36 | call to Atoi | IncorrectNumericConversion.go:208:18:208:36 | call to Atoi : tuple type | IncorrectNumericConversion.go:212:7:212:18 | type conversion | Incorrect conversion of a (arch-dependent)-bit number from strconv.Atoi result to a lower bit size type int8 | +| IncorrectNumericConversion.go:215:18:215:36 | call to Atoi | IncorrectNumericConversion.go:215:18:215:36 | call to Atoi : tuple type | IncorrectNumericConversion.go:219:7:219:19 | type conversion | Incorrect conversion of a (arch-dependent)-bit number from strconv.Atoi result to a lower bit size type int16 | +| IncorrectNumericConversion.go:222:18:222:36 | call to Atoi | IncorrectNumericConversion.go:222:18:222:36 | call to Atoi : tuple type | IncorrectNumericConversion.go:226:7:226:19 | type conversion | Incorrect conversion of a (arch-dependent)-bit number from strconv.Atoi result to a lower bit size type int32 | +| IncorrectNumericConversion.go:229:18:229:36 | call to Atoi | IncorrectNumericConversion.go:229:18:229:36 | call to Atoi : tuple type | IncorrectNumericConversion.go:233:7:233:19 | type conversion | Incorrect conversion of a (arch-dependent)-bit number from strconv.Atoi result to a lower bit size type uint8 | +| IncorrectNumericConversion.go:236:18:236:36 | call to Atoi | IncorrectNumericConversion.go:236:18:236:36 | call to Atoi : tuple type | IncorrectNumericConversion.go:240:7:240:20 | type conversion | Incorrect conversion of a (arch-dependent)-bit number from strconv.Atoi result to a lower bit size type uint16 | +| IncorrectNumericConversion.go:243:18:243:36 | call to Atoi | IncorrectNumericConversion.go:243:18:243:36 | call to Atoi : tuple type | IncorrectNumericConversion.go:247:7:247:20 | type conversion | Incorrect conversion of a (arch-dependent)-bit number from strconv.Atoi result to a lower bit size type uint32 | +| IncorrectNumericConversion.go:250:18:250:36 | call to Atoi | IncorrectNumericConversion.go:250:18:250:36 | call to Atoi : tuple type | IncorrectNumericConversion.go:254:7:254:21 | type conversion | Incorrect conversion of a (arch-dependent)-bit number from strconv.Atoi result to a lower bit size type float32 | +| IncorrectNumericConversion.go:257:18:257:36 | call to Atoi | IncorrectNumericConversion.go:257:18:257:36 | call to Atoi : tuple type | IncorrectNumericConversion.go:262:7:262:18 | type conversion | Incorrect conversion of a (arch-dependent)-bit number from strconv.Atoi result to a lower bit size type uint8 | +| IncorrectNumericConversion.go:266:18:266:36 | call to Atoi | IncorrectNumericConversion.go:266:18:266:36 | call to Atoi : tuple type | IncorrectNumericConversion.go:270:7:270:23 | type conversion | Incorrect conversion of a (arch-dependent)-bit number from strconv.Atoi result to a lower bit size type int16 | diff --git a/ql/test/experimental/CWE-681/IncorrectNumericConversion.go b/ql/test/experimental/CWE-681/IncorrectNumericConversion.go new file mode 100644 index 00000000000..cdabda91a4a --- /dev/null +++ b/ql/test/experimental/CWE-681/IncorrectNumericConversion.go @@ -0,0 +1,355 @@ +package main + +import ( + "math" + "strconv" +) + +func main() { + +} + +type Something struct { +} +type Config struct { +} +type Registry struct { +} + +func LookupTarget(conf *Config, num int32) (int32, error) { + return 567, nil +} +func LookupNumberByName(reg *Registry, name string) (int32, error) { + return 567, nil +} +func lab(s string) (*Something, error) { + num, err := strconv.Atoi(s) + + if err != nil { + number, err := LookupNumberByName(&Registry{}, s) + if err != nil { + return nil, err + } + num = int(number) + } + target, err := LookupTarget(&Config{}, int32(num)) + if err != nil { + return nil, err + } + + // convert the resolved target number back to a string + + s = strconv.Itoa(int(target)) + + return nil, nil +} + +const CustomMaxInt16 = 1<<15 - 1 + +type CustomInt int16 + +func badParseFloat() { + { + parsed, err := strconv.ParseFloat("1.32", 32) + if err != nil { + panic(err) + } + _ = int16(parsed) + } + { + parsed, err := strconv.ParseFloat("1.32", 64) + if err != nil { + panic(err) + } + _ = int32(parsed) + } +} +func badParseInt() { + { + parsed, err := strconv.ParseInt("3456", 10, 16) + if err != nil { + panic(err) + } + _ = int8(parsed) + } + { + parsed, err := strconv.ParseInt("3456", 10, 32) + if err != nil { + panic(err) + } + _ = int16(parsed) + } + { + parsed, err := strconv.ParseInt("3456", 10, 64) + if err != nil { + panic(err) + } + _ = int32(parsed) + } + { + parsed, err := strconv.ParseInt("3456", 10, 0) + if err != nil { + panic(err) + } + _ = int32(parsed) + } +} +func badParseUint() { + { + parsed, err := strconv.ParseUint("3456", 10, 16) + if err != nil { + panic(err) + } + _ = int8(parsed) + } + { + parsed, err := strconv.ParseUint("3456", 10, 32) + if err != nil { + panic(err) + } + _ = int16(parsed) + } + { + parsed, err := strconv.ParseUint("3456", 10, 64) + if err != nil { + panic(err) + } + _ = int32(parsed) + } + { + parsed, err := strconv.ParseUint("3456", 10, 0) + if err != nil { + panic(err) + } + _ = int32(parsed) + } +} + +func goodParseFloat() { + { + parsed, err := strconv.ParseFloat("1.32", 32) + if err != nil { + panic(err) + } + _ = int32(parsed) + } + { + parsed, err := strconv.ParseFloat("1.32", 64) + if err != nil { + panic(err) + } + _ = int64(parsed) + } +} +func goodParseInt() { + { + parsed, err := strconv.ParseInt("3456", 10, 16) + if err != nil { + panic(err) + } + _ = int16(parsed) + } + { + parsed, err := strconv.ParseInt("3456", 10, 32) + if err != nil { + panic(err) + } + _ = int32(parsed) + } + { + parsed, err := strconv.ParseInt("3456", 10, 64) + if err != nil { + panic(err) + } + _ = int64(parsed) + } + { + parsed, err := strconv.ParseInt("3456", 10, 0) + if err != nil { + panic(err) + } + _ = int64(parsed) + } +} +func goodParseUint() { + { + parsed, err := strconv.ParseUint("3456", 10, 16) + if err != nil { + panic(err) + } + _ = int16(parsed) + } + { + parsed, err := strconv.ParseUint("3456", 10, 32) + if err != nil { + panic(err) + } + _ = int32(parsed) + } + { + parsed, err := strconv.ParseUint("3456", 10, 64) + if err != nil { + panic(err) + } + _ = int64(parsed) + } + { + parsed, err := strconv.ParseUint("3456", 10, 0) + if err != nil { + panic(err) + } + _ = int64(parsed) + } +} + +// these should be caught: +func upperBoundIsNOTChecked(input string) { + { + parsed, err := strconv.Atoi(input) + if err != nil { + panic(err) + } + _ = int8(parsed) + } + { + parsed, err := strconv.Atoi(input) + if err != nil { + panic(err) + } + _ = int16(parsed) + } + { + parsed, err := strconv.Atoi(input) + if err != nil { + panic(err) + } + _ = int32(parsed) + } + { + parsed, err := strconv.Atoi(input) + if err != nil { + panic(err) + } + _ = uint8(parsed) + } + { + parsed, err := strconv.Atoi(input) + if err != nil { + panic(err) + } + _ = uint16(parsed) + } + { + parsed, err := strconv.Atoi(input) + if err != nil { + panic(err) + } + _ = uint32(parsed) + } + { + parsed, err := strconv.Atoi(input) + if err != nil { + panic(err) + } + _ = float32(parsed) + } + { + parsed, err := strconv.Atoi(input) + if err != nil { + panic(err) + } + // NOTE: byte is uint8 + _ = byte(parsed) + } + { + // using custom type: + parsed, err := strconv.Atoi(input) + if err != nil { + panic(err) + } + _ = CustomInt(parsed) + } + +} + +// these should NOT be caught: +func upperBoundIsChecked(input string) { + { + parsed, err := strconv.Atoi(input) + if err != nil { + panic(err) + } + if parsed < math.MaxInt8 { + _ = int8(parsed) + } + } + { + parsed, err := strconv.Atoi(input) + if err != nil { + panic(err) + } + if parsed < math.MaxInt16 { + _ = int16(parsed) + } + } + { + parsed, err := strconv.Atoi(input) + if err != nil { + panic(err) + } + if parsed > 0 { + _ = int32(parsed) + } + } + { + parsed, err := strconv.Atoi(input) + if err != nil { + panic(err) + } + if parsed < math.MaxInt32 { + _ = int32(parsed) + } + } + { + parsed, err := strconv.Atoi(input) + if err != nil { + panic(err) + } + if parsed < math.MaxUint8 { + _ = uint8(parsed) + } + } + { + parsed, err := strconv.Atoi(input) + if err != nil { + panic(err) + } + if parsed < math.MaxUint16 { + _ = uint16(parsed) + } + } + { + parsed, err := strconv.Atoi(input) + if err != nil { + panic(err) + } + if parsed < math.MaxUint8 { + _ = byte(parsed) + } + } + { // multiple `and` conditions + parsed, err := strconv.Atoi(input) + if err == nil && 1 == 1 && parsed < math.MaxInt8 { + _ = int8(parsed) + } + } + { // custom maxInt16 + parsed, err := strconv.Atoi(input) + if err != nil { + panic(err) + } + if parsed < CustomMaxInt16 { + _ = int16(parsed) + } + } +} diff --git a/ql/test/experimental/CWE-681/IncorrectNumericConversion.qlref b/ql/test/experimental/CWE-681/IncorrectNumericConversion.qlref new file mode 100644 index 00000000000..884cf918f1c --- /dev/null +++ b/ql/test/experimental/CWE-681/IncorrectNumericConversion.qlref @@ -0,0 +1 @@ +experimental/CWE-681/IncorrectNumericConversion.ql