hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-25 16:55:19 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #12036 from nmouha/patch-1

Browse Source 
CPP: Add query for CVE-2022-37454: Integer addition may overflow inside if statement
This commit is contained in:
Mathias Vorreiter Pedersen
2023-06-05 12:13:27 +01:00
committed by GitHub
parent 06d48dca67 27519ce3ea
commit 52fb00cac3
5 changed files with 174 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
cpp/ql/src/experimental/Security/CWE/CWE-190/IfStatementAdditionOverflow.qhelp Normal file
View File

@@ -0,0 +1,33 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>
Detects <code>if (a+b>c) a=c-b</code>, which incorrectly implements
<code>a = min(a,c-b)</code> if <code>a+b</code> overflows.
</p>
<p>
Also detects variants such as <code>if (b+a>c) a=c-b</code> (swapped
terms in addition), <code>if (a+b>c) { a=c-b }</code> (assignment
inside block), <code>c&lt;a+b</code> (swapped operands), and
<code>&gt;=</code>, <code>&lt;</code>, <code>&lt;=</code> instead of
<code>&gt;</code> (all operators).
</p>
<p>
This integer overflow is the root cause of the buffer overflow in
the SHA-3 reference implementation (CVE-2022-37454).
</p>
</overview>
<recommendation>
<p>
Replace by <code>if (a>c-b) a=c-b</code>. This avoids the overflow
and makes it easy to see that <code>a = min(a,c-b)</code>.
</p>
</recommendation>
<references>
<li>CVE-2022-37454: <a href="https://nvd.nist.gov/vuln/detail/CVE-2022-37454">The Keccak XKCP SHA-3 reference implementation before fdc6fef has an integer overflow and resultant buffer overflow that allows attackers to execute arbitrary code or eliminate expected cryptographic properties. This occurs in the sponge function interface.</a></li>
<li>GitHub Advisory Database: <a href="https://github.com/advisories/GHSA-6w4m-2xhg-2658">CVE-2022-37454: Buffer overflow in sponge queue functions</a></li>
</references>
</qhelp>

42
cpp/ql/src/experimental/Security/CWE/CWE-190/IfStatementAdditionOverflow.ql Normal file
View File

@@ -0,0 +1,42 @@
/**
* @name Integer addition may overflow inside if statement
* @description Writing 'if (a+b>c) a=c-b' incorrectly implements
* 'a = min(a,c-b)' if 'a+b' overflows. This integer
* overflow is the root cause of the buffer overflow
* in the SHA-3 reference implementation (CVE-2022-37454).
* @kind problem
* @problem.severity warning
* @id cpp/if-statement-addition-overflow
* @tags: experimental
* correctness
* security
* external/cwe/cwe-190
*/
import cpp
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
import semmle.code.cpp.valuenumbering.HashCons
import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
import semmle.code.cpp.controlflow.Guards
from
GuardCondition guard, Expr expr, ExprStmt exprstmt, BasicBlock block, AssignExpr assignexpr,
AddExpr addexpr, SubExpr subexpr
where
(guard.ensuresLt(expr, addexpr, 0, block, _) or guard.ensuresLt(addexpr, expr, 0, block, _)) and
addexpr.getUnspecifiedType() instanceof IntegralType and
exprMightOverflowPositively(addexpr) and
block.getANode() = exprstmt and
exprstmt.getExpr() = assignexpr and
assignexpr.getRValue() = subexpr and
(
hashCons(addexpr.getLeftOperand()) = hashCons(assignexpr.getLValue()) and
globalValueNumber(addexpr.getRightOperand()) = globalValueNumber(subexpr.getRightOperand())
or
hashCons(addexpr.getRightOperand()) = hashCons(assignexpr.getLValue()) and
globalValueNumber(addexpr.getLeftOperand()) = globalValueNumber(subexpr.getRightOperand())
) and
globalValueNumber(expr) = globalValueNumber(subexpr.getLeftOperand())
select guard,
"\"if (a+b>c) a=c-b\" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as \"if (a>c-b) a=c-b\" which avoids the overflow.",
addexpr, "addition"

35
cpp/ql/test/experimental/query-tests/Security/CWE/CWE-190/IfStatementAdditionOverflow/IfStatementAdditionOverflow.expected Normal file
View File

@@ -0,0 +1,35 @@
| test.cpp:18:6:18:10 | ... > ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:18:6:18:8 | ... + ... | addition |
| test.cpp:19:6:19:10 | ... > ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:19:6:19:8 | ... + ... | addition |
| test.cpp:20:6:20:10 | ... > ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:20:6:20:8 | ... + ... | addition |
| test.cpp:21:6:21:10 | ... > ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:21:6:21:8 | ... + ... | addition |
| test.cpp:22:6:22:10 | ... > ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:22:8:22:10 | ... + ... | addition |
| test.cpp:23:6:23:10 | ... > ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:23:8:23:10 | ... + ... | addition |
| test.cpp:24:6:24:10 | ... > ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:24:8:24:10 | ... + ... | addition |
| test.cpp:25:6:25:10 | ... > ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:25:8:25:10 | ... + ... | addition |
| test.cpp:27:6:27:11 | ... >= ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:27:6:27:8 | ... + ... | addition |
| test.cpp:28:6:28:11 | ... >= ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:28:6:28:8 | ... + ... | addition |
| test.cpp:29:6:29:11 | ... >= ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:29:6:29:8 | ... + ... | addition |
| test.cpp:30:6:30:11 | ... >= ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:30:6:30:8 | ... + ... | addition |
| test.cpp:31:6:31:11 | ... >= ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:31:9:31:11 | ... + ... | addition |
| test.cpp:32:6:32:11 | ... >= ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:32:9:32:11 | ... + ... | addition |
| test.cpp:33:6:33:11 | ... >= ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:33:9:33:11 | ... + ... | addition |
| test.cpp:34:6:34:11 | ... >= ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:34:9:34:11 | ... + ... | addition |
| test.cpp:36:6:36:10 | ... < ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:36:6:36:8 | ... + ... | addition |
| test.cpp:37:6:37:10 | ... < ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:37:6:37:8 | ... + ... | addition |
| test.cpp:38:6:38:10 | ... < ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:38:6:38:8 | ... + ... | addition |
| test.cpp:39:6:39:10 | ... < ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:39:6:39:8 | ... + ... | addition |
| test.cpp:40:6:40:10 | ... < ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:40:8:40:10 | ... + ... | addition |
| test.cpp:41:6:41:10 | ... < ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:41:8:41:10 | ... + ... | addition |
| test.cpp:42:6:42:10 | ... < ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:42:8:42:10 | ... + ... | addition |
| test.cpp:43:6:43:10 | ... < ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:43:8:43:10 | ... + ... | addition |
| test.cpp:45:6:45:11 | ... <= ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:45:6:45:8 | ... + ... | addition |
| test.cpp:46:6:46:11 | ... <= ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:46:6:46:8 | ... + ... | addition |
| test.cpp:47:6:47:11 | ... <= ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:47:6:47:8 | ... + ... | addition |
| test.cpp:48:6:48:11 | ... <= ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:48:6:48:8 | ... + ... | addition |
| test.cpp:49:6:49:11 | ... <= ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:49:9:49:11 | ... + ... | addition |
| test.cpp:50:6:50:11 | ... <= ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:50:9:50:11 | ... + ... | addition |
| test.cpp:51:6:51:11 | ... <= ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:51:9:51:11 | ... + ... | addition |
| test.cpp:52:6:52:11 | ... <= ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:52:9:52:11 | ... + ... | addition |
| test.cpp:54:6:54:10 | ... > ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:54:6:54:8 | ... + ... | addition |
| test.cpp:61:6:61:11 | ... <= ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:61:6:61:8 | ... + ... | addition |
| test.cpp:62:6:62:11 | ... <= ... | "if (a+b>c) a=c-b" was detected where the $@ may potentially overflow/wraparound. The code can be rewritten as "if (a>c-b) a=c-b" which avoids the overflow. | test.cpp:62:6:62:8 | ... + ... | addition |

1
cpp/ql/test/experimental/query-tests/Security/CWE/CWE-190/IfStatementAdditionOverflow/IfStatementAdditionOverflow.qlref Normal file
View File

@@ -0,0 +1 @@
experimental/Security/CWE/CWE-190/IfStatementAdditionOverflow.ql

63
cpp/ql/test/experimental/query-tests/Security/CWE/CWE-190/IfStatementAdditionOverflow/test.cpp Normal file
View File

@@ -0,0 +1,63 @@
int getAnInt();
double getADouble();
unsigned short getAnUnsignedShort();
void test()
{
int a = getAnInt();
int b = getAnInt();
int c = getAnInt();
int x = getAnInt();
int y = getAnInt();
double d = getADouble();
unsigned short a1 = getAnUnsignedShort();
unsigned short b1 = getAnUnsignedShort();
unsigned short c1 = getAnUnsignedShort();
if (a+b>c) a = c-b; // BAD
if (a+b>c) { a = c-b; } // BAD
if (b+a>c) a = c-b; // BAD
if (b+a>c) { a = c-b; } // BAD
if (c>a+b) a = c-b; // BAD
if (c>a+b) { a = c-b; } // BAD
if (c>b+a) a = c-b; // BAD
if (c>b+a) { a = c-b; } // BAD
if (a+b>=c) a = c-b; // BAD
if (a+b>=c) { a = c-b; } // BAD
if (b+a>=c) a = c-b; // BAD
if (b+a>=c) { a = c-b; } // BAD
if (c>=a+b) a = c-b; // BAD
if (c>=a+b) { a = c-b; } // BAD
if (c>=b+a) a = c-b; // BAD
if (c>=b+a) { a = c-b; } // BAD
if (a+b<c) a = c-b; // BAD
if (a+b<c) { a = c-b; } // BAD
if (b+a<c) a = c-b; // BAD
if (b+a<c) { a = c-b; } // BAD
if (c<a+b) a = c-b; // BAD
if (c<a+b) { a = c-b; } // BAD
if (c<b+a) a = c-b; // BAD
if (c<b+a) { a = c-b; } // BAD
if (a+b<=c) a = c-b; // BAD
if (a+b<=c) { a = c-b; } // BAD
if (b+a<=c) a = c-b; // BAD
if (b+a<=c) { a = c-b; } // BAD
if (c<=a+b) a = c-b; // BAD
if (c<=a+b) { a = c-b; } // BAD
if (c<=b+a) a = c-b; // BAD
if (c<=b+a) { a = c-b; } // BAD
if (a+b>d) a = d-b; // BAD
if (a+(double)b>c) a = c-b; // GOOD
if (a+(-x)>c) a = c-(-y); // GOOD
if (a+b>c) { b++; a = c-b; } // GOOD
if (a+d>c) a = c-d; // GOOD
if (a1+b1>c1) a1 = c1-b1; // GOOD
if (a+b<=c) { /* ... */ } else { a = c-b; } // BAD
if (a+b<=c) { return; } a = c-b; // BAD
}