diff --git a/ql/src/experimental/CWE-369/DivideByZero.qhelp b/ql/src/experimental/CWE-369/DivideByZero.qhelp new file mode 100644 index 00000000000..ae39d1df890 --- /dev/null +++ b/ql/src/experimental/CWE-369/DivideByZero.qhelp @@ -0,0 +1,25 @@ + + + +

+ In Go, dividing an integer by zero leads to a panic, which might interrupt execution of the program and lead to program termination. +

+
+ +

+ Check that all user inputs used as a divisor are non-zero. It is also possible to handle a panic, + generated by division by zero, using the built-in function recover. +

+
+ +

+The following example shows data received from user input being used as a divisor and +possibly causing a divide-by-zero panic. +

+ +

+This can be fixed by testing the divisor against against zero: +

+ +
+
diff --git a/ql/src/experimental/CWE-369/DivideByZero.ql b/ql/src/experimental/CWE-369/DivideByZero.ql new file mode 100644 index 00000000000..5b4518b1c6e --- /dev/null +++ b/ql/src/experimental/CWE-369/DivideByZero.ql @@ -0,0 +1,65 @@ +/** + * @name Divide by zero + * @description Dividing an integer by a user-controlled value may lead to division by zero and an unexpected panic. + * @kind path-problem + * @problem.severity error + * @id go/divide-by-zero + * @tags security + * external/cwe/cwe-369 + */ + +import go +import DataFlow::PathGraph +import semmle.go.dataflow.internal.TaintTrackingUtil + +/** + * A barrier-guard, which represents comparison and equality with zero. + */ +class DivideByZeroSanitizerGuard extends DataFlow::BarrierGuard { + DivideByZeroSanitizerGuard() { + this.(DataFlow::EqualityTestNode).getAnOperand().getNumericValue() = 0 or + this.(DataFlow::RelationalComparisonNode).getAnOperand().getNumericValue() = 0 + } + + override predicate checks(Expr e, boolean branch) { + exists(DataFlow::Node zero, DataFlow::Node checked | + zero.getNumericValue() = 0 and + e = checked.asExpr() and + checked.getType().getUnderlyingType() instanceof IntegerType and + ( + this.(DataFlow::EqualityTestNode).eq(branch.booleanNot(), checked, zero) or + this.(DataFlow::RelationalComparisonNode).leq(branch.booleanNot(), checked, zero, 0) + ) + ) + } +} + +/** + * A taint-tracking configuration for reasoning about division by zero, where divisor is user-controlled and unchecked. + */ +class DivideByZeroCheckConfig extends TaintTracking::Configuration { + DivideByZeroCheckConfig() { this = "DivideByZeroCheckConfig" } + + override predicate isSource(DataFlow::Node source) { source instanceof UntrustedFlowSource } + + override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { + exists(Function f, DataFlow::CallNode cn | cn = f.getACall() | + f.hasQualifiedName("strconv", ["Atoi", "ParseInt", "ParseUint", "ParseFloat"]) and + pred = cn.getArgument(0) and + succ = cn.getResult(0) + ) + } + + override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) { + guard instanceof DivideByZeroSanitizerGuard + } + + override predicate isSink(DataFlow::Node sink) { + sink = DataFlow::exprNode(any(QuoExpr e).getRightOperand()) + } +} + +from DataFlow::PathNode source, DataFlow::PathNode sink, DivideByZeroCheckConfig cfg +where cfg.hasFlowPath(source, sink) +select sink, source, sink, "Variable $@ might be zero leading to a division-by-zero panic.", sink, + sink.getNode().toString() diff --git a/ql/src/experimental/CWE-369/DivideByZeroBad.go b/ql/src/experimental/CWE-369/DivideByZeroBad.go new file mode 100644 index 00000000000..cb943aa82b8 --- /dev/null +++ b/ql/src/experimental/CWE-369/DivideByZeroBad.go @@ -0,0 +1,19 @@ +package main + +import ( + "fmt" + "os" + "strconv" +) + +func main() { + if len(os.Args) < 2 { + fmt.Printf("Usage: ./program value\n") + return + } + val1 := 1337 + value, _ := strconv.Atoi(os.Args[1]) + out := val1 / value + fmt.Println(out) + return +} diff --git a/ql/src/experimental/CWE-369/DivideByZeroGood.go b/ql/src/experimental/CWE-369/DivideByZeroGood.go new file mode 100644 index 00000000000..3b80421322b --- /dev/null +++ b/ql/src/experimental/CWE-369/DivideByZeroGood.go @@ -0,0 +1,23 @@ +package main + +import ( + "fmt" + "os" + "strconv" +) + +func main() { + if len(os.Args) < 2 { + fmt.Printf("Usage: ./program value\n") + return + } + val1 := 1337 + value, _ := strconv.Atoi(os.Args[1]) + if value == 0 { + fmt.Println("Division by zero attempted!") + return + } + out := val1 / value + fmt.Println(out) + return +} diff --git a/ql/test/experimental/CWE-369/DivideByZero.expected b/ql/test/experimental/CWE-369/DivideByZero.expected new file mode 100644 index 00000000000..370d5d099c4 --- /dev/null +++ b/ql/test/experimental/CWE-369/DivideByZero.expected @@ -0,0 +1,31 @@ +edges +| DivideByZero.go:10:12:10:16 | selection of URL : pointer type | DivideByZero.go:12:16:12:20 | value | +| DivideByZero.go:17:12:17:16 | selection of URL : pointer type | DivideByZero.go:18:11:18:24 | type conversion : uint8 | +| DivideByZero.go:18:11:18:24 | type conversion : uint8 | DivideByZero.go:19:16:19:20 | value | +| DivideByZero.go:24:12:24:16 | selection of URL : pointer type | DivideByZero.go:26:16:26:20 | value | +| DivideByZero.go:31:12:31:16 | selection of URL : pointer type | DivideByZero.go:33:16:33:20 | value | +| DivideByZero.go:38:12:38:16 | selection of URL : pointer type | DivideByZero.go:40:16:40:20 | value | +| DivideByZero.go:54:12:54:16 | selection of URL : pointer type | DivideByZero.go:55:11:55:24 | type conversion : uint8 | +| DivideByZero.go:55:11:55:24 | type conversion : uint8 | DivideByZero.go:57:17:57:21 | value | +nodes +| DivideByZero.go:10:12:10:16 | selection of URL : pointer type | semmle.label | selection of URL : pointer type | +| DivideByZero.go:12:16:12:20 | value | semmle.label | value | +| DivideByZero.go:17:12:17:16 | selection of URL : pointer type | semmle.label | selection of URL : pointer type | +| DivideByZero.go:18:11:18:24 | type conversion : uint8 | semmle.label | type conversion : uint8 | +| DivideByZero.go:19:16:19:20 | value | semmle.label | value | +| DivideByZero.go:24:12:24:16 | selection of URL : pointer type | semmle.label | selection of URL : pointer type | +| DivideByZero.go:26:16:26:20 | value | semmle.label | value | +| DivideByZero.go:31:12:31:16 | selection of URL : pointer type | semmle.label | selection of URL : pointer type | +| DivideByZero.go:33:16:33:20 | value | semmle.label | value | +| DivideByZero.go:38:12:38:16 | selection of URL : pointer type | semmle.label | selection of URL : pointer type | +| DivideByZero.go:40:16:40:20 | value | semmle.label | value | +| DivideByZero.go:54:12:54:16 | selection of URL : pointer type | semmle.label | selection of URL : pointer type | +| DivideByZero.go:55:11:55:24 | type conversion : uint8 | semmle.label | type conversion : uint8 | +| DivideByZero.go:57:17:57:21 | value | semmle.label | value | +#select +| DivideByZero.go:12:16:12:20 | value | DivideByZero.go:10:12:10:16 | selection of URL : pointer type | DivideByZero.go:12:16:12:20 | value | Variable $@ might be zero leading to a division-by-zero panic. | DivideByZero.go:12:16:12:20 | value | value | +| DivideByZero.go:19:16:19:20 | value | DivideByZero.go:17:12:17:16 | selection of URL : pointer type | DivideByZero.go:19:16:19:20 | value | Variable $@ might be zero leading to a division-by-zero panic. | DivideByZero.go:19:16:19:20 | value | value | +| DivideByZero.go:26:16:26:20 | value | DivideByZero.go:24:12:24:16 | selection of URL : pointer type | DivideByZero.go:26:16:26:20 | value | Variable $@ might be zero leading to a division-by-zero panic. | DivideByZero.go:26:16:26:20 | value | value | +| DivideByZero.go:33:16:33:20 | value | DivideByZero.go:31:12:31:16 | selection of URL : pointer type | DivideByZero.go:33:16:33:20 | value | Variable $@ might be zero leading to a division-by-zero panic. | DivideByZero.go:33:16:33:20 | value | value | +| DivideByZero.go:40:16:40:20 | value | DivideByZero.go:38:12:38:16 | selection of URL : pointer type | DivideByZero.go:40:16:40:20 | value | Variable $@ might be zero leading to a division-by-zero panic. | DivideByZero.go:40:16:40:20 | value | value | +| DivideByZero.go:57:17:57:21 | value | DivideByZero.go:54:12:54:16 | selection of URL : pointer type | DivideByZero.go:57:17:57:21 | value | Variable $@ might be zero leading to a division-by-zero panic. | DivideByZero.go:57:17:57:21 | value | value | diff --git a/ql/test/experimental/CWE-369/DivideByZero.go b/ql/test/experimental/CWE-369/DivideByZero.go new file mode 100644 index 00000000000..613479981b1 --- /dev/null +++ b/ql/test/experimental/CWE-369/DivideByZero.go @@ -0,0 +1,80 @@ +package main + +import ( + "fmt" + "net/http" + "strconv" +) + +func myHandler1(w http.ResponseWriter, r *http.Request) { + param1 := r.URL.Query()["param1"][0] + value, _ := strconv.Atoi(param1) + out := 1337 / value + fmt.Println(out) +} + +func myHandler2(w http.ResponseWriter, r *http.Request) { + param1 := r.URL.Query()["param1"][0] + value := int(param1[0]) + out := 1337 / value + fmt.Println(out) +} + +func myHandler3(w http.ResponseWriter, r *http.Request) { + param1 := r.URL.Query()["param1"][0] + value, _ := strconv.ParseInt(param1, 10, 64) + out := 1337 / value + fmt.Println(out) +} + +func myHandler4(w http.ResponseWriter, r *http.Request) { + param1 := r.URL.Query()["param1"][0] + value, _ := strconv.ParseFloat(param1, 32) + out := 1337 / value + fmt.Println(out) +} + +func myHandler5(w http.ResponseWriter, r *http.Request) { + param1 := r.URL.Query()["param1"][0] + value, _ := strconv.ParseUint(param1, 10, 64) + out := 1337 / value + fmt.Println(out) +} + +func myHandler6(w http.ResponseWriter, r *http.Request) { + param1 := r.URL.Query()["param1"][0] + value := int(param1[0]) + if value != 0 { + out := 1337 / value + fmt.Println(out) + } +} + +func myHandler7(w http.ResponseWriter, r *http.Request) { + param1 := r.URL.Query()["param1"][0] + value := int(param1[0]) + if value >= 0 { + out := 1337 / value + fmt.Println(out) + } +} + +func myHandler8(w http.ResponseWriter, r *http.Request) { + param1 := r.URL.Query()["param1"][0] + value, _ := strconv.ParseInt(param1, 10, 64) + if value > 0 { + out := 1337 / value + fmt.Println(out) + } +} + +func myHandler9(w http.ResponseWriter, r *http.Request) { + param1 := r.URL.Query()["param1"][0] + value, _ := strconv.ParseInt(param1, 10, 64) + if value == 0 { + fmt.Println(param1) + return + } + out := 1337 / value + fmt.Println(out) +} diff --git a/ql/test/experimental/CWE-369/DivideByZero.qlref b/ql/test/experimental/CWE-369/DivideByZero.qlref new file mode 100644 index 00000000000..6aae6de95d7 --- /dev/null +++ b/ql/test/experimental/CWE-369/DivideByZero.qlref @@ -0,0 +1 @@ +experimental/CWE-369/DivideByZero.ql