Merge pull request #116 from gagliardetto/CWE-681

CWE 681

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)
+}

ql/src/experimental/CWE-681/IncorrectNumericConversion.qhelp

@@ -0,0 +1,65 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+<overview>
+<p>
+	If a numeric value string is parsed using <code>strconv.Atoi</code> into an int, and subsequently that int
+	is converted into another type of a smaller size, the result can produce unexpected values.
+</p>
+<p>
+	This also applies to the results of <code>strconv.ParseFloat</code>, <code>strconv.ParseInt</code>,
+	and <code>strconv.ParseUint</code> when the specified size is larger than the size of the
+	type that number is converted to.
+</p>
+</overview>
+<recommendation>
+<p>
+	If you need to parse numeric values with specific bit sizes, avoid <code>strconv.Atoi</code>, and instead
+	use the functions specific to each type (<code>strconv.ParseFloat</code>, <code>strconv.ParseInt</code>,
+	<code>strconv.ParseUint</code>) that also allow to specify the wanted bit size.
+</p>
+<p>
+	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.
+</p>
+<p>
+	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).
+</p>
+</recommendation>
+<example>
+<p>
+	In the first example, assume that an input string is passed to <code>parseAllocateBad1</code> function,
+	parsed by <code>strconv.Atoi</code>, and then converted into an <code>int32</code> type:
+</p>
+<sample src="IncorrectNumericConversion.go"/>
+<p>
+	The bounds are not checked, so this means that if the provided number is greater than the maximum value of type <code>int32</code>,
+	the resulting value from the conversion will be different from the actual provided value.
+</p>
+<p>
+	To avoid unexpected values, you should either use the other functions provided by the <code>strconv</code>
+	package to parse the specific types and bit sizes as shown in the
+	<code>parseAllocateGood2</code> function; or check bounds as in the <code>parseAllocateGood1</code>
+	function.
+</p>
+<sample src="IncorrectNumericConversionGood.go"/>
+</example>
+<example>
+<p>
+	In the second example, assume that an input string is passed to <code>parseAllocateBad2</code> function,
+	parsed by <code>strconv.ParseInt</code> with a bit size set to 64, and then converted into an <code>int32</code> type:
+</p>
+<sample src="IncorrectNumericConversion.go"/>
+<p>
+	If the provided number is greater than the maximum value of type <code>int32</code>, the resulting value from the conversion will be
+	different from the actual provided value.
+</p>
+<p>
+	To avoid unexpected values, you should specify the correct bit size as in <code>parseAllocateGood3</code>;
+	or check bounds before making the conversion as in <code>parseAllocateGood4</code>.
+</p>
+<sample src="IncorrectNumericConversionGood.go"/>
+</example>
+</qhelp>

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()

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
+}

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 |

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)
+		}
+	}
+}

ql/test/experimental/CWE-681/IncorrectNumericConversion.qlref

@@ -0,0 +1 @@
+experimental/CWE-681/IncorrectNumericConversion.ql