diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-369/DivideByZeroUsingReturnValue.cpp b/cpp/ql/src/experimental/Security/CWE/CWE-369/DivideByZeroUsingReturnValue.cpp new file mode 100644 index 00000000000..1d970a47678 --- /dev/null +++ b/cpp/ql/src/experimental/Security/CWE/CWE-369/DivideByZeroUsingReturnValue.cpp @@ -0,0 +1,7 @@ + +... + a = getc(f); + if (a < 123) ret = 123/a; // BAD +... + if (a != 0) ret = 123/a; // GOOD +... diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-369/DivideByZeroUsingReturnValue.qhelp b/cpp/ql/src/experimental/Security/CWE/CWE-369/DivideByZeroUsingReturnValue.qhelp new file mode 100644 index 00000000000..39ae034f46c --- /dev/null +++ b/cpp/ql/src/experimental/Security/CWE/CWE-369/DivideByZeroUsingReturnValue.qhelp @@ -0,0 +1,23 @@ + + + +

Possible cases of division by zero when using the return value from functions.

+ +
+ + +

The following example shows the use of a function with an error when using the return value and without an error.

+ + +
+ + +
  • + CERT Coding Standard: + INT33-C. Ensure that division and remainder operations do not result in divide-by-zero errors - SEI CERT C Coding Standard - Confluence. +
  • + +
    +
    \ No newline at end of file diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-369/DivideByZeroUsingReturnValue.ql b/cpp/ql/src/experimental/Security/CWE/CWE-369/DivideByZeroUsingReturnValue.ql new file mode 100644 index 00000000000..d55743c592b --- /dev/null +++ b/cpp/ql/src/experimental/Security/CWE/CWE-369/DivideByZeroUsingReturnValue.ql @@ -0,0 +1,274 @@ +/** + * @name Divide by zero using return value + * @description Possible cases of division by zero when using the return value from functions. + * @kind problem + * @id cpp/divide-by-zero-using-return-value + * @problem.severity warning + * @precision medium + * @tags correctness + * security + * external/cwe/cwe-369 + */ + +import cpp +import semmle.code.cpp.valuenumbering.GlobalValueNumbering +import semmle.code.cpp.controlflow.Guards + +/** Holds if function `fn` can return a value equal to value `val` */ +predicate mayBeReturnValue(Function fn, float val) { + exists(Expr tmpExp, ReturnStmt rs | + tmpExp.getValue().toFloat() = val and + rs.getEnclosingFunction() = fn and + ( + globalValueNumber(rs.getExpr()) = globalValueNumber(tmpExp) + or + exists(AssignExpr ae | + ae.getLValue().(VariableAccess).getTarget() = + globalValueNumber(rs.getExpr()).getAnExpr().(VariableAccess).getTarget() and + globalValueNumber(ae.getRValue()) = globalValueNumber(tmpExp) + ) + or + exists(Initializer it | + globalValueNumber(it.getExpr()) = globalValueNumber(tmpExp) and + it.getDeclaration().(Variable).getAnAccess().getTarget() = + globalValueNumber(rs.getExpr()).getAnExpr().(VariableAccess).getTarget() + ) + ) + ) +} + +/** Holds if function `fn` can return a value equal zero */ +predicate mayBeReturnZero(Function fn) { + mayBeReturnValue(fn, 0) + or + fn.hasName([ + "iswalpha", "iswlower", "iswprint", "iswspace", "iswblank", "iswupper", "iswcntrl", + "iswctype", "iswalnum", "iswgraph", "iswxdigit", "iswdigit", "iswpunct", "isblank", "isupper", + "isgraph", "isalnum", "ispunct", "islower", "isspace", "isprint", "isxdigit", "iscntrl", + "isdigit", "isalpha", "timespec_get", "feof", "atomic_is_lock_free", + "atomic_compare_exchange", "thrd_equal", "isfinite", "islessequal", "isnan", "isgreater", + "signbit", "isinf", "islessgreater", "isnormal", "isless", "isgreaterequal", "isunordered", + "ferror" + ]) + or + fn.hasName([ + "thrd_sleep", "feenv", "feholdexcept", "feclearexcept", "feexceptflag", "feupdateenv", + "remove", "fflush", "setvbuf", "fgetpos", "fsetpos", "fclose", "rename", "fseek", "raise" + ]) + or + fn.hasName(["tss_get", "gets"]) + or + fn.hasName(["getc", "atoi"]) +} + +/** Gets the Guard which compares the expression `bound` */ +pragma[inline] +GuardCondition checkByValue(Expr bound, Expr val) { + exists(GuardCondition gc | + ( + gc.ensuresEq(bound, val, _, _, _) or + gc.ensuresEq(val, bound, _, _, _) or + gc.ensuresLt(bound, val, _, _, _) or + gc.ensuresLt(val, bound, _, _, _) or + gc = globalValueNumber(bound).getAnExpr() + ) and + result = gc + ) +} + +/** Holds if there are no comparisons between the value returned by possible function calls `compArg` and the value `valArg`, or when these comparisons do not exclude equality to the value `valArg`. */ +pragma[inline] +predicate compareFunctionWithValue(Expr guardExp, Function compArg, Expr valArg) { + not exists(Expr exp | + exp.getAChild*() = globalValueNumber(compArg.getACallToThisFunction()).getAnExpr() and + checkByValue(exp, valArg).controls(guardExp.getBasicBlock(), _) + ) + or + exists(GuardCondition gc | + ( + gc.ensuresEq(globalValueNumber(compArg.getACallToThisFunction()).getAnExpr(), valArg, 0, + guardExp.getBasicBlock(), true) + or + gc.ensuresEq(valArg, globalValueNumber(compArg.getACallToThisFunction()).getAnExpr(), 0, + guardExp.getBasicBlock(), true) + or + gc.ensuresLt(globalValueNumber(compArg.getACallToThisFunction()).getAnExpr(), valArg, 0, + guardExp.getBasicBlock(), false) + or + gc.ensuresLt(valArg, globalValueNumber(compArg.getACallToThisFunction()).getAnExpr(), 0, + guardExp.getBasicBlock(), false) + ) + or + exists(Expr exp | + exp.getValue().toFloat() > valArg.getValue().toFloat() and + gc.ensuresLt(globalValueNumber(compArg.getACallToThisFunction()).getAnExpr(), exp, 0, + guardExp.getBasicBlock(), true) + or + exp.getValue().toFloat() < valArg.getValue().toFloat() and + gc.ensuresLt(exp, globalValueNumber(compArg.getACallToThisFunction()).getAnExpr(), 0, + guardExp.getBasicBlock(), true) + ) + ) + or + valArg.getValue().toFloat() = 0 and + exists(NotExpr ne, IfStmt ifne | + ne.getOperand() = globalValueNumber(compArg.getACallToThisFunction()).getAnExpr() and + ifne.getCondition() = ne and + ifne.getThen().getAChild*() = guardExp + ) +} + +/** Wraping predicate for call `compareFunctionWithValue`. */ +pragma[inline] +predicate checkConditions1(Expr div, Function fn, float changeInt) { + exists(Expr val | + val.getEnclosingFunction() = fn and + val.getValue().toFloat() = changeInt and + compareFunctionWithValue(div, fn, val) + ) +} + +/** Holds if there are no comparisons between the value `compArg` and the value `valArg`, or when these comparisons do not exclude equality to the value `valArg`. */ +pragma[inline] +predicate compareExprWithValue(Expr guardExp, Expr compArg, Expr valArg) { + not exists(Expr exp | + exp.getAChild*() = globalValueNumber(compArg).getAnExpr() and + checkByValue(exp, valArg).controls(guardExp.getBasicBlock(), _) + ) + or + exists(GuardCondition gc | + ( + gc.ensuresEq(globalValueNumber(compArg).getAnExpr(), valArg, 0, guardExp.getBasicBlock(), true) + or + gc.ensuresEq(valArg, globalValueNumber(compArg).getAnExpr(), 0, guardExp.getBasicBlock(), true) + or + gc.ensuresLt(globalValueNumber(compArg).getAnExpr(), valArg, 0, guardExp.getBasicBlock(), + false) + or + gc.ensuresLt(valArg, globalValueNumber(compArg).getAnExpr(), 0, guardExp.getBasicBlock(), + false) + ) + or + exists(Expr exp | + exp.getValue().toFloat() > valArg.getValue().toFloat() and + gc.ensuresLt(globalValueNumber(compArg).getAnExpr(), exp, 0, guardExp.getBasicBlock(), true) + or + exp.getValue().toFloat() < valArg.getValue().toFloat() and + gc.ensuresLt(exp, globalValueNumber(compArg).getAnExpr(), 0, guardExp.getBasicBlock(), true) + ) + ) + or + valArg.getValue().toFloat() = 0 and + exists(NotExpr ne, IfStmt ifne | + ne.getOperand() = globalValueNumber(compArg).getAnExpr() and + ifne.getCondition() = ne and + ifne.getThen().getAChild*() = guardExp + ) +} + +/** Wraping predicate for call `compareExprWithValue`. */ +pragma[inline] +predicate checkConditions2(Expr div, Expr divVal, float changeInt2) { + exists(Expr val | + ( + val.getEnclosingFunction() = + div.getEnclosingFunction().getACallToThisFunction().getEnclosingFunction() or + val.getEnclosingFunction() = div.getEnclosingFunction() + ) and + val.getValue().toFloat() = changeInt2 and + compareExprWithValue(div, divVal, val) + ) +} + +/** Gets the value of the difference or summand from the expression `src`. */ +float getValueOperand(Expr src, Expr e1, Expr e2) { + src.(SubExpr).hasOperands(e1, e2) and + result = e2.getValue().toFloat() + or + src.(AddExpr).hasOperands(e1, e2) and + result = -e2.getValue().toFloat() +} + +/** Function the return of the expression `e1` and the multiplication operands, or the left operand of division if `e1` contains a multiplication or division, respectively. */ +Expr getMulDivOperand(Expr e1) { + result = e1 or + result = e1.(MulExpr).getAnOperand() or + result = e1.(DivExpr).getLeftOperand() +} + +/** The class that defines possible variants of the division expression or the search for the remainder. */ +class MyDiv extends Expr { + MyDiv() { + this instanceof DivExpr or + this instanceof RemExpr or + this instanceof AssignDivExpr or + this instanceof AssignRemExpr + } + + Expr getRV() { + result = this.(AssignArithmeticOperation).getRValue() or + result = this.(BinaryArithmeticOperation).getRightOperand() + } +} + +from Expr exp, string msg, Function fn, GVN findVal, float changeInt, MyDiv div +where + findVal = globalValueNumber(fn.getACallToThisFunction()) and + ( + // Look for divide-by-zero operations possible due to the return value of the function `fn`. + checkConditions1(div, fn, changeInt) and + ( + // Function return value can be zero. + mayBeReturnZero(fn) and + getMulDivOperand(globalValueNumber(div.getRV()).getAnExpr()) = findVal.getAnExpr() and + changeInt = 0 + or + // Denominator can be sum or difference. + changeInt = getValueOperand(div.getRV(), findVal.getAnExpr(), _) and + mayBeReturnValue(fn, changeInt) + ) and + exp = div and + msg = + "Can lead to division by 0, since the function " + fn.getName() + " can return a value " + + changeInt.toString() + "." + or + // Search for situations where division by zero is possible inside the `divFn` function if the passed argument can be equal to a certain value. + exists(int posArg, Expr divVal, FunctionCall divFc, float changeInt2 | + // Division is associated with the function argument. + exists(Function divFn | + divFn.getParameter(posArg).getAnAccess() = divVal and + divVal.getEnclosingStmt() = div.getEnclosingStmt() and + divFc = divFn.getACallToThisFunction() + ) and + ( + divVal = div.getRV() and + divFc.getArgument(posArg) != findVal.getAnExpr() and + ( + // Function return value can be zero. + mayBeReturnZero(fn) and + getMulDivOperand(globalValueNumber(divFc.getArgument(posArg)).getAnExpr()) = + findVal.getAnExpr() and + changeInt = 0 and + changeInt2 = 0 + or + // Denominator can be sum or difference. + changeInt = getValueOperand(divFc.getArgument(posArg), findVal.getAnExpr(), _) and + mayBeReturnValue(fn, changeInt) and + changeInt2 = 0 + ) + or + // Look for a situation where the difference or subtraction is considered as an argument, and it can be used in the same way. + changeInt = getValueOperand(div.getRV(), divVal, _) and + changeInt2 = changeInt and + mayBeReturnValue(fn, changeInt) and + divFc.getArgument(posArg) = findVal.getAnExpr() + ) and + checkConditions2(div, divVal, changeInt2) and + checkConditions1(divFc, fn, changeInt) and + exp = divFc and + msg = + "Can lead to division by 0, since the function " + fn.getName() + " can return a value " + + changeInt.toString() + "." + ) + ) +select exp, msg diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-369/semmle/tests/DivideByZeroUsingReturnValue.expected b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-369/semmle/tests/DivideByZeroUsingReturnValue.expected new file mode 100644 index 00000000000..b8296505cb8 --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-369/semmle/tests/DivideByZeroUsingReturnValue.expected @@ -0,0 +1,27 @@ +| test.cpp:47:24:47:31 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. | +| test.cpp:48:15:48:34 | ... / ... | Can lead to division by 0, since the function getSize2 can return a value 0. | +| test.cpp:53:10:53:17 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. | +| test.cpp:65:15:65:22 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. | +| test.cpp:68:15:68:22 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. | +| test.cpp:71:9:71:16 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. | +| test.cpp:74:9:74:16 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. | +| test.cpp:77:21:77:28 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. | +| test.cpp:79:25:79:32 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. | +| test.cpp:81:24:81:31 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. | +| test.cpp:128:10:128:16 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. | +| test.cpp:135:10:135:16 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. | +| test.cpp:141:10:141:23 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. | +| test.cpp:153:12:153:19 | ... / ... | Can lead to division by 0, since the function getSize can return a value 0. | +| test.cpp:172:3:172:12 | ... /= ... | Can lead to division by 0, since the function getSize can return a value 0. | +| test.cpp:173:3:173:12 | ... %= ... | Can lead to division by 0, since the function getSize can return a value 0. | +| test.cpp:187:10:187:17 | ... / ... | Can lead to division by 0, since the function getSizeFloat can return a value 0. | +| test.cpp:199:12:199:25 | ... / ... | Can lead to division by 0, since the function getSize can return a value -1. | +| test.cpp:202:12:202:25 | ... / ... | Can lead to division by 0, since the function getSize can return a value 1. | +| test.cpp:205:10:205:23 | ... / ... | Can lead to division by 0, since the function getSize can return a value 1. | +| test.cpp:210:10:210:23 | ... / ... | Can lead to division by 0, since the function getSize can return a value 3. | +| test.cpp:258:3:258:10 | call to badMyDiv | Can lead to division by 0, since the function getSize can return a value 0. | +| test.cpp:259:3:259:10 | call to badMyDiv | Can lead to division by 0, since the function getSize can return a value 2. | +| test.cpp:260:3:260:13 | call to badMySubDiv | Can lead to division by 0, since the function getSize can return a value 3. | +| test.cpp:263:5:263:15 | call to badMySubDiv | Can lead to division by 0, since the function getSize can return a value 3. | +| test.cpp:273:5:273:12 | call to badMyDiv | Can lead to division by 0, since the function getSize can return a value 3. | +| test.cpp:275:5:275:12 | call to badMyDiv | Can lead to division by 0, since the function getSize can return a value -1. | diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-369/semmle/tests/DivideByZeroUsingReturnValue.qlref b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-369/semmle/tests/DivideByZeroUsingReturnValue.qlref new file mode 100644 index 00000000000..e134a5229da --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-369/semmle/tests/DivideByZeroUsingReturnValue.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-369/DivideByZeroUsingReturnValue.ql diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-369/semmle/tests/test.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-369/semmle/tests/test.cpp new file mode 100644 index 00000000000..882f6618485 --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-369/semmle/tests/test.cpp @@ -0,0 +1,278 @@ +typedef struct {} +FILE; +int getc(FILE * stream); + +int getSize(int type) { + int st; + switch (type) { + case 1: + st = 1; + break; + case 2: + st = 2; + break; + case 3: + st = 3; + break; + case 4: + st = -1; + break; + default: + st = 0; + break; + } + return st; +} +int getSize2(int type) { + int st = 0; + switch (type) { + case 1: + st = 1; + break; + case 2: + st = 2; + break; + case 3: + st = 3; + break; + case 4: + st = -1; + break; + } + return st; +} + +int badTestf1(int type, int met) { + int is = getSize(type); + if (met == 1) return 123 / is; // BAD + else return 123 / getSize2(type); // BAD +} +int badTestf2(int type) { + int is; + is = getSize(type); + return 123 / is; // BAD +} + +int badTestf3(int type, int met) { + int is; + is = getSize(type); + switch (met) { + case 1: + if (is >= 0) return 123 / is; // BAD [NOT DETECTED] + case 2: + if (0 == is) return 123 / is; // BAD [NOT DETECTED] + case 3: + if (!is & 123 / is) // BAD + return 123; + case 4: + if (!is | 123 / is) // BAD + return 123; + case 5: + if (123 / is || !is) // BAD + return 123; + case 6: + if (123 / is && !is) // BAD + return 123; + case 7: + if (!is) return 123 / is; // BAD + case 8: + if (is > -1) return 123 / is; // BAD + case 9: + if (is < 2) return 123 / is; // BAD + } + if (is != 0) return -1; + if (is == 0) type += 1; + return 123 / is; // BAD [NOT DETECTED] +} + +int goodTestf3(int type, int met) { + int is = getSize(type); + if (is == 0) return -1; + switch (met) { + case 1: + if (is < 0) return 123 / is; // GOOD + case 2: + if (!is && 123 / is) // GOOD + return 123; + case 3: + if (!is || 123 / is) // GOOD + return 123; + case 8: + if (is < -1) return 123 / is; // GOOD + case 9: + if (is > 2) return 123 / is; // GOOD + } + return 123 / is; +} + +int goodTestf3a(int type, int met) { + int is = getSize(type); + switch (met) { + case 1: + if (is < 0) + return 123 / is; // GOOD + case 2: + if (!is && 123 / is) // GOOD + return 123; + case 3: + if (!is || 123 / is) // GOOD + return 123; + } + return 1; +} + +int badTestf4(int type) { + int is = getSize(type); + int d; + d = type * is; + return 123 / d; // BAD +} + +int badTestf5(int type) { + int is = getSize(type); + int d; + d = is / type; + return 123 / d; // BAD +} +int badTestf6(int type) { + int is = getSize(type); + int d; + d = is / type; + return type * 123 / d; // BAD +} + +int badTestf7(int type, int met) { + int is = getSize(type); + if (is == 0) goto quit; + switch (met) { + case 1: + if (is < 0) + return 123 / is; // GOOD + } + quit: + return 123 / is; // BAD +} + +int goodTestf7(int type, int met) { + int is = getSize(type); + if (is == 0) goto quit2; + if (is == 0.) return -1; + switch (met) { + case 1: + if (is < 0.) + return 123 / is; // GOOD + } + return 123 / is; // GOOD + quit2: + return -1; +} + +int badTestf8(int type) { + int is = getSize(type); + type /= is; // BAD + type %= is; // BAD + return type; +} + +float getSizeFloat(float type) { + float st; + if (type) + st = 1.0; + else + st = 0.0; + return st; +} +float badTestf9(float type) { + float is = getSizeFloat(type); + return 123 / is; // BAD +} +float goodTestf9(float type) { + float is = getSizeFloat(type); + if (is == 0.0) return -1; + return 123 / is; // GOOD +} + +int badTestf10(int type) { + int out = type; + int is = getSize(type); + if (is > -2) { + out /= 123 / (is + 1); // BAD + } + if (is > 0) { + return 123 / (is - 1); // BAD + } + if (is <= 0) return 0; + return 123 / (is - 1); // BAD + return 0; +} +int badTestf11(int type) { + int is = getSize(type); + return 123 / (is - 3); // BAD +} + +int goodTestf11(int type) { + int is = getSize(type); + if (is > 1) { + return 123 / (is - 1); // GOOD + } else { + return 0; + } +} + +int badTestf12(FILE * f) { + int a; + int ret = -1; + a = getc(f); + if (a == 0) ret = 123 / a; // BAD [NOT DETECTED] + return ret; +} + +int goodTestf12(FILE * f) { + int a; + int ret = -1; + a = getc(f); + if (a != 0) ret = 123 / a; // GOOD + return ret; +} + +int badMyDiv(int type, int is) { + type /= is; + type %= is; + return type; +} + +int goodMyDiv(int type, int is) { + if (is == 0) return -1; + type /= is; + type %= is; + return type; +} +int badMySubDiv(int type, int is) { + type /= (is - 3); + type %= (is + 1); + return type; +} + +void badTestf13(int type) { + int is = getSize(type); + badMyDiv(type, is); // BAD + badMyDiv(type, is - 2); // BAD + badMySubDiv(type, is); // BAD + goodMyDiv(type, is); // GOOD + if (is < 5) + badMySubDiv(type, is); // BAD + if (is < 0) + badMySubDiv(type, is); // BAD [NOT DETECTED] + if (is > 5) + badMySubDiv(type, is); // GOOD + if (is == 0) + badMyDiv(type, is); // BAD + if (is > 0) + badMyDiv(type, is); // GOOD + if (is < 5) + badMyDiv(type, is - 3); // BAD + if (is < 0) + badMyDiv(type, is + 1); // BAD + if (is > 5) + badMyDiv(type, is - 3); // GOOD +}