зеркало из https://github.com/github/codeql.git
Merge pull request #492 from geoffw0/offsetuse
Approved by dave-bartolomeo
This commit is contained in:
Коммит
62db19bee7
|
@ -15,6 +15,7 @@
|
||||||
|
|
||||||
| **Query** | **Expected impact** | **Change** |
|
| **Query** | **Expected impact** | **Change** |
|
||||||
|----------------------------|------------------------|------------------------------------------------------------------|
|
|----------------------------|------------------------|------------------------------------------------------------------|
|
||||||
|
| Array offset used before range check | More results and fewer false positive results | The query now recognizes array accesses in different positions within the expression. False positives where the range is checked before and after the array access have been fixed. |
|
||||||
| Empty branch of conditional | Fewer false positive results | The query now recognizes commented blocks more reliably. |
|
| Empty branch of conditional | Fewer false positive results | The query now recognizes commented blocks more reliably. |
|
||||||
| Expression has no effect | Fewer false positive results | Expressions in template instantiations are now excluded from this query. |
|
| Expression has no effect | Fewer false positive results | Expressions in template instantiations are now excluded from this query. |
|
||||||
| Global could be static | Fewer false positive results | Variables with declarations in header files are now excluded from this query. |
|
| Global could be static | Fewer false positive results | Variables with declarations in header files are now excluded from this query. |
|
||||||
|
|
|
@ -5,6 +5,7 @@
|
||||||
* @kind problem
|
* @kind problem
|
||||||
* @id cpp/offset-use-before-range-check
|
* @id cpp/offset-use-before-range-check
|
||||||
* @problem.severity warning
|
* @problem.severity warning
|
||||||
|
* @precision medium
|
||||||
* @tags reliability
|
* @tags reliability
|
||||||
* security
|
* security
|
||||||
* external/cwe/cwe-120
|
* external/cwe/cwe-120
|
||||||
|
@ -13,10 +14,29 @@
|
||||||
|
|
||||||
import cpp
|
import cpp
|
||||||
|
|
||||||
from Variable v, LogicalAndExpr andexpr, ArrayExpr access, LTExpr rangecheck
|
predicate beforeArrayAccess(Variable v, ArrayExpr access, Expr before) {
|
||||||
where access.getArrayOffset() = v.getAnAccess()
|
exists(LogicalAndExpr andexpr |
|
||||||
and andexpr.getLeftOperand().getAChild() = access
|
access.getArrayOffset() = v.getAnAccess() and
|
||||||
and andexpr.getRightOperand() = rangecheck
|
andexpr.getRightOperand().getAChild*() = access and
|
||||||
and rangecheck.getLeftOperand() = v.getAnAccess()
|
andexpr.getLeftOperand() = before
|
||||||
and not access.isInMacroExpansion()
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
predicate afterArrayAccess(Variable v, ArrayExpr access, Expr after) {
|
||||||
|
exists(LogicalAndExpr andexpr |
|
||||||
|
access.getArrayOffset() = v.getAnAccess() and
|
||||||
|
andexpr.getLeftOperand().getAChild*() = access and
|
||||||
|
andexpr.getRightOperand() = after
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
from Variable v, ArrayExpr access, LTExpr rangecheck
|
||||||
|
where
|
||||||
|
afterArrayAccess(v, access, rangecheck) and
|
||||||
|
rangecheck.getLeftOperand() = v.getAnAccess() and
|
||||||
|
not access.isInMacroExpansion() and
|
||||||
|
not exists(LTExpr altcheck |
|
||||||
|
beforeArrayAccess(v, access, altcheck) and
|
||||||
|
altcheck.getLeftOperand() = v.getAnAccess()
|
||||||
|
)
|
||||||
select access, "This use of offset '" + v.getName() + "' should follow the $@.", rangecheck, "range check"
|
select access, "This use of offset '" + v.getName() + "' should follow the $@.", rangecheck, "range check"
|
||||||
|
|
|
@ -0,0 +1,6 @@
|
||||||
|
| test.cpp:11:10:11:18 | access to array | This use of offset 'i' should follow the $@. | test.cpp:11:32:11:45 | ... < ... | range check |
|
||||||
|
| test.cpp:15:7:15:15 | access to array | This use of offset 'i' should follow the $@. | test.cpp:15:29:15:42 | ... < ... | range check |
|
||||||
|
| test.cpp:27:7:27:15 | access to array | This use of offset 'i' should follow the $@. | test.cpp:27:39:27:52 | ... < ... | range check |
|
||||||
|
| test.cpp:39:8:39:16 | access to array | This use of offset 'i' should follow the $@. | test.cpp:39:30:39:47 | ... < ... | range check |
|
||||||
|
| test.cpp:44:7:44:15 | access to array | This use of offset 'i' should follow the $@. | test.cpp:44:22:44:35 | ... < ... | range check |
|
||||||
|
| test.cpp:47:7:47:15 | access to array | This use of offset 'i' should follow the $@. | test.cpp:47:33:47:46 | ... < ... | range check |
|
|
@ -0,0 +1 @@
|
||||||
|
Best Practices/Likely Errors/OffsetUseBeforeRangeCheck.ql
|
|
@ -0,0 +1,50 @@
|
||||||
|
|
||||||
|
void test(char *buffer, int bufferSize)
|
||||||
|
{
|
||||||
|
int i;
|
||||||
|
|
||||||
|
// skip whitespace
|
||||||
|
i = 0;
|
||||||
|
while ((i < bufferSize) && (buffer[i] == ' ')) { i++; } // GOOD
|
||||||
|
|
||||||
|
i = 0;
|
||||||
|
while ((buffer[i] == ' ') && (i < bufferSize)) { i++; } // BAD
|
||||||
|
|
||||||
|
// check for 'x'
|
||||||
|
if ((i < bufferSize) && (buffer[i] == 'x')) {} // GOOD
|
||||||
|
if ((buffer[i] == 'x') && (i < bufferSize)) {} // BAD
|
||||||
|
|
||||||
|
if ((bufferSize > i) && (buffer[i] == 'x')) {} // GOOD
|
||||||
|
if ((buffer[i] == 'x') && (bufferSize > i)) {} // BAD [NOT DETECTED]
|
||||||
|
|
||||||
|
if ((i <= bufferSize - 1) && (buffer[i] == 'x')) {} // GOOD
|
||||||
|
if ((buffer[i] == 'x') && (i <= bufferSize - 1)) {} // BAD [NOT DETECTED]
|
||||||
|
|
||||||
|
if ((bufferSize >= i + 1) && (buffer[i] == 'x')) {} // GOOD
|
||||||
|
if ((buffer[i] == 'x') && (bufferSize >= i + 1)) {} // BAD [NOT DETECTED]
|
||||||
|
|
||||||
|
if ((i < bufferSize) && (true) && (buffer[i] == 'x')) {} // GOOD
|
||||||
|
if ((buffer[i] == 'x') && (true) && (i < bufferSize)) {} // BAD
|
||||||
|
|
||||||
|
if ((i < bufferSize - 1) && (buffer[i + 1] == 'x')) {} // GOOD
|
||||||
|
if ((buffer[i + 1] == 'x') && (i < bufferSize - 1)) {} // BAD [NOT DETECTED]
|
||||||
|
|
||||||
|
if ((i < bufferSize) && (buffer[i] == 'x') && (i < bufferSize - 1)) {} // GOOD
|
||||||
|
if ((i < bufferSize) && ((buffer[i] == 'x') && (i < bufferSize - 1))) {} // GOOD
|
||||||
|
if ((i < bufferSize + 1) && (buffer[i] == 'x') && (i < bufferSize)) {} // BAD [NOT DETECTED]
|
||||||
|
if ((i < bufferSize + 1) && ((buffer[i] == 'x') && (i < bufferSize))) {} // BAD [NOT DETECTED]
|
||||||
|
|
||||||
|
// look for 'ab'
|
||||||
|
for (i = 0; i < bufferSize; i++) {
|
||||||
|
if ((buffer[i] == 'a') && (i < bufferSize - 1) && (buffer[i + 1] == 'b')) // GOOD [FALSE POSITIVE]
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
|
||||||
|
if ((i < bufferSize) && (buffer[i])) {} // GOOD
|
||||||
|
if ((buffer[i]) && (i < bufferSize)) {} // BAD
|
||||||
|
|
||||||
|
if ((i < bufferSize) && (buffer[i] + 1 == 'x')) {} // GOOD
|
||||||
|
if ((buffer[i] + 1 == 'x') && (i < bufferSize)) {} // BAD
|
||||||
|
|
||||||
|
if ((buffer != 0) && (i < bufferSize)) {} // GOOD
|
||||||
|
}
|
Загрузка…
Ссылка в новой задаче