mirror of
https://github.com/github/codeql.git
synced 2026-04-28 18:25:24 +02:00
Merge pull request #7386 from github/redsun82/cpp-overrunning-write-precision-split
C++: split `cpp/overrunning-write` into two
This commit is contained in:
@@ -10,10 +10,22 @@ private import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
|
||||
private import semmle.code.cpp.rangeanalysis.RangeAnalysisUtils
|
||||
|
||||
private newtype TBufferWriteEstimationReason =
|
||||
TNoSpecifiedEstimateReason() or
|
||||
TUnspecifiedEstimateReason() or
|
||||
TTypeBoundsAnalysis() or
|
||||
TWidenedValueFlowAnalysis() or
|
||||
TValueFlowAnalysis()
|
||||
|
||||
private predicate gradeToReason(int grade, TBufferWriteEstimationReason reason) {
|
||||
// when combining reasons, lower grade takes precedence
|
||||
grade = 0 and reason = TUnspecifiedEstimateReason()
|
||||
or
|
||||
grade = 1 and reason = TTypeBoundsAnalysis()
|
||||
or
|
||||
grade = 2 and reason = TWidenedValueFlowAnalysis()
|
||||
or
|
||||
grade = 3 and reason = TValueFlowAnalysis()
|
||||
}
|
||||
|
||||
/**
|
||||
* A reason for a specific buffer write size estimate.
|
||||
*/
|
||||
@@ -32,7 +44,13 @@ abstract class BufferWriteEstimationReason extends TBufferWriteEstimationReason
|
||||
* Combine estimate reasons. Used to give a reason for the size of a format string
|
||||
* conversion given reasons coming from its individual specifiers.
|
||||
*/
|
||||
abstract BufferWriteEstimationReason combineWith(BufferWriteEstimationReason other);
|
||||
BufferWriteEstimationReason combineWith(BufferWriteEstimationReason other) {
|
||||
exists(int grade, int otherGrade |
|
||||
gradeToReason(grade, this) and gradeToReason(otherGrade, other)
|
||||
|
|
||||
if otherGrade < grade then result = other else result = this
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -40,16 +58,10 @@ abstract class BufferWriteEstimationReason extends TBufferWriteEstimationReason
|
||||
* classes derived from BufferWrite and overriding `getMaxData/0` still work with the
|
||||
* queries as intended.
|
||||
*/
|
||||
class NoSpecifiedEstimateReason extends BufferWriteEstimationReason, TNoSpecifiedEstimateReason {
|
||||
override string toString() { result = "NoSpecifiedEstimateReason" }
|
||||
class UnspecifiedEstimateReason extends BufferWriteEstimationReason, TUnspecifiedEstimateReason {
|
||||
override string toString() { result = "UnspecifiedEstimateReason" }
|
||||
|
||||
override string getDescription() { result = "no reason specified" }
|
||||
|
||||
override BufferWriteEstimationReason combineWith(BufferWriteEstimationReason other) {
|
||||
// this reason should not be used in format specifiers, so it should not be combined
|
||||
// with other reasons
|
||||
none()
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -60,9 +72,24 @@ class TypeBoundsAnalysis extends BufferWriteEstimationReason, TTypeBoundsAnalysi
|
||||
override string toString() { result = "TypeBoundsAnalysis" }
|
||||
|
||||
override string getDescription() { result = "based on type bounds" }
|
||||
}
|
||||
|
||||
override BufferWriteEstimationReason combineWith(BufferWriteEstimationReason other) {
|
||||
other != TNoSpecifiedEstimateReason() and result = TTypeBoundsAnalysis()
|
||||
/**
|
||||
* The estimation comes from non trivial bounds found via actual flow analysis,
|
||||
* but a widening aproximation might have been used for variables in loops.
|
||||
* For example
|
||||
* ```
|
||||
* for (int i = 0; i < 10; ++i) {
|
||||
* int j = i + i;
|
||||
* //... <- estimation done here based on j
|
||||
* }
|
||||
* ```
|
||||
*/
|
||||
class WidenedValueFlowAnalysis extends BufferWriteEstimationReason, TWidenedValueFlowAnalysis {
|
||||
override string toString() { result = "WidenedValueFlowAnalysis" }
|
||||
|
||||
override string getDescription() {
|
||||
result = "based on flow analysis of value bounds with a widening approximation"
|
||||
}
|
||||
}
|
||||
|
||||
@@ -80,10 +107,6 @@ class ValueFlowAnalysis extends BufferWriteEstimationReason, TValueFlowAnalysis
|
||||
override string toString() { result = "ValueFlowAnalysis" }
|
||||
|
||||
override string getDescription() { result = "based on flow analysis of value bounds" }
|
||||
|
||||
override BufferWriteEstimationReason combineWith(BufferWriteEstimationReason other) {
|
||||
other != TNoSpecifiedEstimateReason() and result = other
|
||||
}
|
||||
}
|
||||
|
||||
class PrintfFormatAttribute extends FormatAttribute {
|
||||
@@ -359,6 +382,23 @@ private int lengthInBase10(float f) {
|
||||
result = f.log10().floor() + 1
|
||||
}
|
||||
|
||||
bindingset[expr]
|
||||
private BufferWriteEstimationReason getEstimationReasonForIntegralExpression(Expr expr) {
|
||||
// we consider the range analysis non trivial if it
|
||||
// * constrained non-trivially both sides of a signed value, or
|
||||
// * constrained non-trivially the positive side of an unsigned value
|
||||
// expr should already be given as getFullyConverted
|
||||
if
|
||||
upperBound(expr) < exprMaxVal(expr) and
|
||||
(exprMinVal(expr) >= 0 or lowerBound(expr) > exprMinVal(expr))
|
||||
then
|
||||
// next we check whether the estimate may have been widened
|
||||
if upperBoundMayBeWidened(expr)
|
||||
then result = TWidenedValueFlowAnalysis()
|
||||
else result = TValueFlowAnalysis()
|
||||
else result = TTypeBoundsAnalysis()
|
||||
}
|
||||
|
||||
/**
|
||||
* A class to represent format strings that occur as arguments to invocations of formatting functions.
|
||||
*/
|
||||
@@ -1157,12 +1197,10 @@ class FormatLiteral extends Literal {
|
||||
1 + lengthInBase10(2.pow(this.getIntegralDisplayType(n).getSize() * 8 - 1)) and
|
||||
// The second case uses range analysis to deduce a length that's shorter than the length
|
||||
// of the number -2^31.
|
||||
exists(Expr arg, float lower, float upper, float typeLower, float typeUpper |
|
||||
exists(Expr arg, float lower, float upper |
|
||||
arg = this.getUse().getConversionArgument(n) and
|
||||
lower = lowerBound(arg.getFullyConverted()) and
|
||||
upper = upperBound(arg.getFullyConverted()) and
|
||||
typeLower = exprMinVal(arg.getFullyConverted()) and
|
||||
typeUpper = exprMaxVal(arg.getFullyConverted())
|
||||
upper = upperBound(arg.getFullyConverted())
|
||||
|
|
||||
valueBasedBound =
|
||||
max(int cand |
|
||||
@@ -1179,11 +1217,9 @@ class FormatLiteral extends Literal {
|
||||
else cand = lengthInBase10(upper)
|
||||
)
|
||||
) and
|
||||
(
|
||||
if lower > typeLower or upper < typeUpper
|
||||
then reason = TValueFlowAnalysis()
|
||||
else reason = TTypeBoundsAnalysis()
|
||||
)
|
||||
// we don't want to call this on `arg.getFullyConverted()` as we want
|
||||
// to detect non-trivial range analysis without taking into account up-casting
|
||||
reason = getEstimationReasonForIntegralExpression(arg)
|
||||
) and
|
||||
len = valueBasedBound.minimum(typeBasedBound)
|
||||
)
|
||||
@@ -1195,12 +1231,10 @@ class FormatLiteral extends Literal {
|
||||
typeBasedBound = lengthInBase10(2.pow(this.getIntegralDisplayType(n).getSize() * 8) - 1) and
|
||||
// The second case uses range analysis to deduce a length that's shorter than
|
||||
// the length of the number 2^31 - 1.
|
||||
exists(Expr arg, float lower, float upper, float typeLower, float typeUpper |
|
||||
exists(Expr arg, float lower, float upper |
|
||||
arg = this.getUse().getConversionArgument(n) and
|
||||
lower = lowerBound(arg.getFullyConverted()) and
|
||||
upper = upperBound(arg.getFullyConverted()) and
|
||||
typeLower = exprMinVal(arg.getFullyConverted()) and
|
||||
typeUpper = exprMaxVal(arg.getFullyConverted())
|
||||
upper = upperBound(arg.getFullyConverted())
|
||||
|
|
||||
valueBasedBound =
|
||||
lengthInBase10(max(float cand |
|
||||
@@ -1210,11 +1244,9 @@ class FormatLiteral extends Literal {
|
||||
or
|
||||
cand = upper
|
||||
)) and
|
||||
(
|
||||
if lower > typeLower or upper < typeUpper
|
||||
then reason = TValueFlowAnalysis()
|
||||
else reason = TTypeBoundsAnalysis()
|
||||
)
|
||||
// we don't want to call this on `arg.getFullyConverted()` as we want
|
||||
// to detect non-trivial range analysis without taking into account up-casting
|
||||
reason = getEstimationReasonForIntegralExpression(arg)
|
||||
) and
|
||||
len = valueBasedBound.minimum(typeBasedBound)
|
||||
)
|
||||
|
||||
@@ -76,7 +76,7 @@ abstract class BufferWrite extends Expr {
|
||||
* can be found), specifying the reason for the estimation.
|
||||
*/
|
||||
int getMaxData(BufferWriteEstimationReason reason) {
|
||||
reason instanceof NoSpecifiedEstimateReason and result = getMaxData()
|
||||
reason instanceof UnspecifiedEstimateReason and result = getMaxData()
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -18,8 +18,5 @@
|
||||
<p>To fix the problem, either the second argument to <code>snprintf</code> should be changed to 80, or the buffer extended to 256 characters. A further improvement is to use a preprocessor define so that the size is only specified in one place, potentially preventing future recurrence of this issue.</p>
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
|
||||
</references>
|
||||
<include src="OverrunWriteReferences.inc.qhelp" />
|
||||
</qhelp>
|
||||
|
||||
@@ -1,9 +1,9 @@
|
||||
void sayHello()
|
||||
void sayHello(uint32_t userId)
|
||||
{
|
||||
char buffer[10];
|
||||
char buffer[18];
|
||||
|
||||
// BAD: this message overflows the buffer
|
||||
strcpy(buffer, "Hello, world!");
|
||||
// BAD: this message overflows the buffer if userId >= 10000
|
||||
sprintf(buffer, "Hello, user %d!", userId);
|
||||
|
||||
MessageBox(hWnd, buffer, "New Message", MB_OK);
|
||||
}
|
||||
@@ -6,30 +6,20 @@
|
||||
<p>The program performs a buffer copy or write operation with no upper limit on the size of the copy, and it appears that certain inputs will cause a buffer overflow to occur in this case. In addition to causing program instability, techniques exist which may allow an attacker to use this vulnerability to execute arbitrary code.</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
<p>Always control the length of buffer copy and buffer write operations. <code>strncpy</code> should be used over <code>strcpy</code>, <code>snprintf</code> over <code>sprintf</code>, and in other cases 'n-variant' functions should be preferred.</p>
|
||||
|
||||
</recommendation>
|
||||
<include src="OverrunWriteRecommendation.inc.qhelp" />
|
||||
<example>
|
||||
<sample src="OverrunWrite.c" />
|
||||
|
||||
<p>In this example, the call to <code>strcpy</code> copies a message of 14 characters (including the terminating null) into a buffer with space for just 10 characters. As such, the last four characters overflow the buffer resulting in undefined behavior.</p>
|
||||
<p>In this example, the call to <code>sprintf</code> writes a message of 14 characters (including the terminating null) plus the length of the string conversion of `userId` into a buffer with space for just 18 characters. As such, if `userId` is greater or equal to `10000`, the last characters overflow the buffer resulting in undefined behavior.</p>
|
||||
|
||||
<p>To fix this issue three changes should be made:</p>
|
||||
<p>To fix this issue these changes should be made:</p>
|
||||
<ul>
|
||||
<li>Control the size of the buffer using a preprocessor define.</li>
|
||||
<li>Replace the call to <code>strcpy</code> with <code>strncpy</code>, specifying the define as the maximum length to copy. This will prevent the buffer overflow.</li>
|
||||
<li>Consider increasing the buffer size, say to 20 characters, so that the message is displayed correctly.</li>
|
||||
<li>Control the size of the buffer by declaring it with a compile time constant.</li>
|
||||
<li>Preferably, replace the call to <code>sprintf</code> with <code>snprintf</code>, using the defined constant size of the buffer or `sizeof(buffer)` as maximum length to write. This will prevent the buffer overflow.</li>
|
||||
<li>Optionally, if `userId` is expected to be less than `10000`, then return or throw an error if `userId` is out of bounds.</li>
|
||||
<li>Otherwise, consider increasing the buffer size to at least 25 characters, so that the message is displayed correctly regardless of the value of `userId`.</li>
|
||||
</ul>
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
<li>CERT C Coding Standard: <a href="https://www.securecoding.cert.org/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator">STR31-C. Guarantee that storage for strings has sufficient space for character data and the null terminator</a>.</li>
|
||||
|
||||
|
||||
<!-- LocalWords: preprocessor CWE STR
|
||||
-->
|
||||
|
||||
</references>
|
||||
<include src="OverrunWriteReferences.inc.qhelp" />
|
||||
</qhelp>
|
||||
|
||||
@@ -21,12 +21,14 @@ import semmle.code.cpp.commons.Alloc
|
||||
* See CWE-120/UnboundedWrite.ql for a summary of CWE-120 alert cases.
|
||||
*/
|
||||
|
||||
from BufferWrite bw, Expr dest, int destSize, int estimated
|
||||
from BufferWrite bw, Expr dest, int destSize, int estimated, BufferWriteEstimationReason reason
|
||||
where
|
||||
not bw.hasExplicitLimit() and // has no explicit size limit
|
||||
dest = bw.getDest() and
|
||||
destSize = getBufferSize(dest, _) and
|
||||
estimated = bw.getMaxDataLimited(_) and
|
||||
estimated = bw.getMaxDataLimited(reason) and
|
||||
// we exclude ValueFlowAnalysis as it is reported in cpp/very-likely-overruning-write
|
||||
not reason instanceof ValueFlowAnalysis and
|
||||
// we can deduce that too much data may be copied (even without
|
||||
// long '%f' conversions)
|
||||
estimated > destSize
|
||||
|
||||
@@ -6,30 +6,19 @@
|
||||
<p>The program performs a buffer copy or write operation that includes one or more float to string conversions (i.e. the %f format specifier), which may overflow the destination buffer if extreme inputs are given. In addition to causing program instability, techniques exist which may allow an attacker to use this vulnerability to execute arbitrary code.</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
<p>Always control the length of buffer copy and buffer write operations. <code>strncpy</code> should be used over <code>strcpy</code>, <code>snprintf</code> over <code>sprintf</code>, and in other cases 'n-variant' functions should be preferred.</p>
|
||||
|
||||
</recommendation>
|
||||
<include src="OverrunWriteRecommendation.inc.qhelp" />
|
||||
<example>
|
||||
<sample src="OverrunWriteFloat.c" />
|
||||
|
||||
<p>In this example, the call to <code>sprintf</code> contains a %f format specifier. Though a 256 character buffer has been allowed, it is not sufficient for the most extreme floating point inputs. For example the representation of double value 1e304 (that is 1 with 304 zeroes after it) will overflow a buffer of this length.</p>
|
||||
<p>In this example, the call to <code>sprintf</code> contains a <code>%f</code> format specifier. Though a 256 character buffer has been allowed, it is not sufficient for the most extreme floating point inputs. For example the representation of double value 1e304 (that is 1 with 304 zeroes after it) will overflow a buffer of this length.</p>
|
||||
|
||||
<p>To fix this issue three changes should be made:</p>
|
||||
<ul>
|
||||
<li>Control the size of the buffer using a preprocessor define.</li>
|
||||
<li>Replace the call to <code>sprintf</code> with <code>snprintf</code>, specifying the define as the maximum length to copy. This will prevent the buffer overflow.</li>
|
||||
<li>Consider using the %g format specifier instead of %f.</li>
|
||||
<li>Consider using the <code>%g</code> format specifier instead of <code>%f</code>.</li>
|
||||
</ul>
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
<li>CERT C Coding
|
||||
Standard: <a href="https://www.securecoding.cert.org/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator">STR31-C. Guarantee
|
||||
that storage for strings has sufficient space for character data and
|
||||
the null terminator</a>.</li>
|
||||
|
||||
|
||||
</references>
|
||||
<include src="OverrunWriteReferences.inc.qhelp" />
|
||||
</qhelp>
|
||||
|
||||
@@ -0,0 +1,8 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<recommendation>
|
||||
<p>Always control the length of buffer copy and buffer write operations. <code>strncpy</code> should be used over <code>strcpy</code>, <code>snprintf</code> over <code>sprintf</code>, and in other cases 'n-variant' functions should be preferred.</p>
|
||||
</recommendation>
|
||||
</qhelp>
|
||||
@@ -0,0 +1,14 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<references>
|
||||
|
||||
<li>CERT C Coding Standard: <a href="https://www.securecoding.cert.org/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator">STR31-C. Guarantee that storage for strings has sufficient space for character data and the null terminator</a>.</li>
|
||||
<li>CERT C++ Coding Standard: <a href="https://www.securecoding.cert.org/confluence/display/cplusplus/STR50-CPP.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator">STR50-CPP. Guarantee that storage for strings has sufficient space for character data and the null terminator</a>.</li>
|
||||
|
||||
<!-- LocalWords: preprocessor CWE STR
|
||||
-->
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
@@ -6,10 +6,7 @@
|
||||
<p>The program performs a buffer copy or write operation with no upper limit on the size of the copy. An unexpectedly long input that reaches this code will cause the buffer to overflow. In addition to causing program instability, techniques exist which may allow an attacker to use this vulnerability to execute arbitrary code.</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
<p>Always control the length of buffer copy and buffer write operations. <code>strncpy</code> should be used over <code>strcpy</code>, <code>snprintf</code> over <code>sprintf</code> etc. In general 'n-variant' functions should be preferred.</p>
|
||||
|
||||
</recommendation>
|
||||
<include src="OverrunWriteRecommendation.inc.qhelp" />
|
||||
<example>
|
||||
<sample src="UnboundedWrite.c" />
|
||||
|
||||
@@ -18,13 +15,5 @@
|
||||
<p>To fix the problem the call to <code>sprintf</code> should be replaced with <code>snprintf</code>, specifying a maximum length of 80 characters.</p>
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
<li>CERT C++ Coding Standard: <a href="https://www.securecoding.cert.org/confluence/display/cplusplus/STR50-CPP.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator">STR50-CPP. Guarantee that storage for strings has sufficient space for character data and the null terminator</a>.</li>
|
||||
|
||||
|
||||
<!-- LocalWords: CWE STR
|
||||
-->
|
||||
|
||||
</references>
|
||||
<include src="OverrunWriteReferences.inc.qhelp" />
|
||||
</qhelp>
|
||||
|
||||
@@ -33,7 +33,7 @@ import TaintedWithPath
|
||||
* hasExplicitLimit() exists(getMaxData()) exists(getBufferSize(bw.getDest(), _))) handled by
|
||||
* NO NO either UnboundedWrite.ql isUnboundedWrite()
|
||||
* NO YES NO UnboundedWrite.ql isMaybeUnboundedWrite()
|
||||
* NO YES YES OverrunWrite.ql, OverrunWriteFloat.ql
|
||||
* NO YES YES VeryLikelyOverrunWrite.ql, OverrunWrite.ql, OverrunWriteFloat.ql
|
||||
* YES either YES BadlyBoundedWrite.ql
|
||||
* YES either NO (assumed OK)
|
||||
*/
|
||||
|
||||
14
cpp/ql/src/Security/CWE/CWE-120/VeryLikelyOverrunWrite.c
Normal file
14
cpp/ql/src/Security/CWE/CWE-120/VeryLikelyOverrunWrite.c
Normal file
@@ -0,0 +1,14 @@
|
||||
int sayHello(uint32_t userId)
|
||||
{
|
||||
char buffer[17];
|
||||
|
||||
if (userId > 9999) return USER_ID_OUT_OF_BOUNDS;
|
||||
|
||||
// BAD: this message overflows the buffer if userId >= 1000,
|
||||
// as no space for the null terminator was accounted for
|
||||
sprintf(buffer, "Hello, user %d!", userId);
|
||||
|
||||
MessageBox(hWnd, buffer, "New Message", MB_OK);
|
||||
|
||||
return SUCCESS;
|
||||
}
|
||||
24
cpp/ql/src/Security/CWE/CWE-120/VeryLikelyOverrunWrite.qhelp
Normal file
24
cpp/ql/src/Security/CWE/CWE-120/VeryLikelyOverrunWrite.qhelp
Normal file
@@ -0,0 +1,24 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<overview>
|
||||
<p>The program performs a buffer copy or write operation with no upper limit on the size of the copy. By analyzing the bounds of the expressions involved, it appears that certain inputs will cause a buffer overflow to occur in this case. In addition to causing program instability, techniques exist which may allow an attacker to use this vulnerability to execute arbitrary code.</p>
|
||||
|
||||
</overview>
|
||||
<include src="OverrunWriteRecommendation.inc.qhelp" />
|
||||
<example>
|
||||
<sample src="VeryLikelyOverrunWrite.c" />
|
||||
|
||||
<p>In this example, the call to <code>sprintf</code> writes a message of 14 characters (including the terminating null) plus the length of the string conversion of `userId` into a buffer with space for just 17 characters. While `userId` is checked to occupy no more than 4 characters when converted, there is no space in the buffer for the terminating null character if `userId >= 1000`. In this case, the null character overflows the buffer resulting in undefined behavior.</p>
|
||||
|
||||
<p>To fix this issue these changes should be made:</p>
|
||||
<ul>
|
||||
<li>Control the size of the buffer by declaring it with a compile time constant.</li>
|
||||
<li>Preferably, replace the call to <code>sprintf</code> with <code>snprintf</code>, using the defined constant size of the buffer or `sizeof(buffer)` as maximum length to write. This will prevent the buffer overflow.</li>
|
||||
<li>Increasing the buffer size to account for the full range of `userId` and the terminating null character.</li>
|
||||
</ul>
|
||||
|
||||
</example>
|
||||
<include src="OverrunWriteReferences.inc.qhelp" />
|
||||
</qhelp>
|
||||
34
cpp/ql/src/Security/CWE/CWE-120/VeryLikelyOverrunWrite.ql
Normal file
34
cpp/ql/src/Security/CWE/CWE-120/VeryLikelyOverrunWrite.ql
Normal file
@@ -0,0 +1,34 @@
|
||||
/**
|
||||
* @name Likely overrunning write
|
||||
* @description Buffer write operations that do not control the length
|
||||
* of data written may overflow
|
||||
* @kind problem
|
||||
* @problem.severity error
|
||||
* @security-severity 9.3
|
||||
* @precision high
|
||||
* @id cpp/very-likely-overrunning-write
|
||||
* @tags reliability
|
||||
* security
|
||||
* external/cwe/cwe-120
|
||||
* external/cwe/cwe-787
|
||||
* external/cwe/cwe-805
|
||||
*/
|
||||
|
||||
import semmle.code.cpp.security.BufferWrite
|
||||
import semmle.code.cpp.commons.Alloc
|
||||
|
||||
/*
|
||||
* See CWE-120/UnboundedWrite.ql for a summary of CWE-120 alert cases.
|
||||
*/
|
||||
|
||||
from BufferWrite bw, Expr dest, int destSize, int estimated, ValueFlowAnalysis reason
|
||||
where
|
||||
not bw.hasExplicitLimit() and // has no explicit size limit
|
||||
dest = bw.getDest() and
|
||||
destSize = getBufferSize(dest, _) and
|
||||
estimated = bw.getMaxDataLimited(reason) and
|
||||
// we can deduce from non-trivial range analysis that too much data may be copied
|
||||
estimated > destSize
|
||||
select bw,
|
||||
"This '" + bw.getBWDesc() + "' operation requires " + estimated +
|
||||
" bytes but the destination is only " + destSize + " bytes."
|
||||
@@ -0,0 +1,4 @@
|
||||
---
|
||||
category: newQuery
|
||||
---
|
||||
* A new `cpp/very-likely-overruning-write` query has been added to the default query suite for C/C++. The query reports some results that were formerly flagged by `cpp/overruning-write`.
|
||||
@@ -0,0 +1 @@
|
||||
Security/CWE/CWE-120/VeryLikelyOverrunWrite.ql
|
||||
@@ -0,0 +1 @@
|
||||
Security/CWE/CWE-120/VeryLikelyOverrunWrite.ql
|
||||
@@ -1,21 +0,0 @@
|
||||
| tests2.cpp:17:3:17:8 | call to wcscpy | This 'call to wcscpy' operation requires 12 bytes but the destination is only 8 bytes. |
|
||||
| tests2.cpp:22:3:22:8 | call to wcscpy | This 'call to wcscpy' operation requires 16 bytes but the destination is only 12 bytes. |
|
||||
| tests2.cpp:27:3:27:8 | call to wcscpy | This 'call to wcscpy' operation requires 20 bytes but the destination is only 16 bytes. |
|
||||
| tests2.cpp:31:3:31:8 | call to wcscpy | This 'call to wcscpy' operation requires 24 bytes but the destination is only 20 bytes. |
|
||||
| tests2.cpp:36:3:36:8 | call to wcscpy | This 'call to wcscpy' operation requires 28 bytes but the destination is only 24 bytes. |
|
||||
| tests2.cpp:41:3:41:8 | call to wcscpy | This 'call to wcscpy' operation requires 32 bytes but the destination is only 28 bytes. |
|
||||
| tests2.cpp:46:3:46:8 | call to wcscpy | This 'call to wcscpy' operation requires 36 bytes but the destination is only 32 bytes. |
|
||||
| tests.c:54:3:54:9 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 10 bytes. |
|
||||
| tests.c:58:3:58:9 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 10 bytes. |
|
||||
| tests.c:62:17:62:24 | buffer10 | This 'scanf string argument' operation requires 11 bytes but the destination is only 10 bytes. |
|
||||
| tests.c:63:17:63:24 | buffer10 | This 'scanf string argument' operation requires 12 bytes but the destination is only 10 bytes. |
|
||||
| tests.c:86:3:86:8 | call to strcpy | This 'call to strcpy' operation requires 6 bytes but the destination is only 5 bytes. |
|
||||
| tests.c:93:3:93:8 | call to strcpy | This 'call to strcpy' operation requires 6 bytes but the destination is only 5 bytes. |
|
||||
| tests.c:120:3:120:9 | call to sprintf | This 'call to sprintf' operation requires 17 bytes but the destination is only 1 bytes. |
|
||||
| tests.c:121:3:121:9 | call to sprintf | This 'call to sprintf' operation requires 17 bytes but the destination is only 16 bytes. |
|
||||
| tests.c:136:2:136:8 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 10 bytes. |
|
||||
| unions.c:26:2:26:7 | call to strcpy | This 'call to strcpy' operation requires 21 bytes but the destination is only 16 bytes. |
|
||||
| unions.c:27:2:27:7 | call to strcpy | This 'call to strcpy' operation requires 21 bytes but the destination is only 15 bytes. |
|
||||
| unions.c:27:2:27:7 | call to strcpy | This 'call to strcpy' operation requires 21 bytes but the destination is only 16 bytes. |
|
||||
| unions.c:32:2:32:7 | call to strcpy | This 'call to strcpy' operation requires 31 bytes but the destination is only 25 bytes. |
|
||||
| var_size_struct.cpp:22:3:22:8 | call to strcpy | This 'call to strcpy' operation requires 10 bytes but the destination is only 9 bytes. |
|
||||
|
||||
@@ -0,0 +1,21 @@
|
||||
| tests2.cpp:17:3:17:8 | call to wcscpy | This 'call to wcscpy' operation requires 12 bytes but the destination is only 8 bytes. |
|
||||
| tests2.cpp:22:3:22:8 | call to wcscpy | This 'call to wcscpy' operation requires 16 bytes but the destination is only 12 bytes. |
|
||||
| tests2.cpp:27:3:27:8 | call to wcscpy | This 'call to wcscpy' operation requires 20 bytes but the destination is only 16 bytes. |
|
||||
| tests2.cpp:31:3:31:8 | call to wcscpy | This 'call to wcscpy' operation requires 24 bytes but the destination is only 20 bytes. |
|
||||
| tests2.cpp:36:3:36:8 | call to wcscpy | This 'call to wcscpy' operation requires 28 bytes but the destination is only 24 bytes. |
|
||||
| tests2.cpp:41:3:41:8 | call to wcscpy | This 'call to wcscpy' operation requires 32 bytes but the destination is only 28 bytes. |
|
||||
| tests2.cpp:46:3:46:8 | call to wcscpy | This 'call to wcscpy' operation requires 36 bytes but the destination is only 32 bytes. |
|
||||
| tests.c:54:3:54:9 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 10 bytes. |
|
||||
| tests.c:58:3:58:9 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 10 bytes. |
|
||||
| tests.c:62:17:62:24 | buffer10 | This 'scanf string argument' operation requires 11 bytes but the destination is only 10 bytes. |
|
||||
| tests.c:63:17:63:24 | buffer10 | This 'scanf string argument' operation requires 12 bytes but the destination is only 10 bytes. |
|
||||
| tests.c:86:3:86:8 | call to strcpy | This 'call to strcpy' operation requires 6 bytes but the destination is only 5 bytes. |
|
||||
| tests.c:93:3:93:8 | call to strcpy | This 'call to strcpy' operation requires 6 bytes but the destination is only 5 bytes. |
|
||||
| tests.c:120:3:120:9 | call to sprintf | This 'call to sprintf' operation requires 17 bytes but the destination is only 1 bytes. |
|
||||
| tests.c:121:3:121:9 | call to sprintf | This 'call to sprintf' operation requires 17 bytes but the destination is only 16 bytes. |
|
||||
| tests.c:136:2:136:8 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 10 bytes. |
|
||||
| unions.c:26:2:26:7 | call to strcpy | This 'call to strcpy' operation requires 21 bytes but the destination is only 16 bytes. |
|
||||
| unions.c:27:2:27:7 | call to strcpy | This 'call to strcpy' operation requires 21 bytes but the destination is only 15 bytes. |
|
||||
| unions.c:27:2:27:7 | call to strcpy | This 'call to strcpy' operation requires 21 bytes but the destination is only 16 bytes. |
|
||||
| unions.c:32:2:32:7 | call to strcpy | This 'call to strcpy' operation requires 31 bytes but the destination is only 25 bytes. |
|
||||
| var_size_struct.cpp:22:3:22:8 | call to strcpy | This 'call to strcpy' operation requires 10 bytes but the destination is only 9 bytes. |
|
||||
@@ -0,0 +1 @@
|
||||
Security/CWE/CWE-120/VeryLikelyOverrunWrite.ql
|
||||
@@ -1,19 +1,8 @@
|
||||
| tests.cpp:258:2:258:8 | call to sprintf | This 'call to sprintf' operation requires 17 bytes but the destination is only 10 bytes. |
|
||||
| tests.cpp:259:2:259:8 | call to sprintf | This 'call to sprintf' operation requires 17 bytes but the destination is only 10 bytes. |
|
||||
| tests.cpp:272:2:272:8 | call to sprintf | This 'call to sprintf' operation requires 9 bytes but the destination is only 8 bytes. |
|
||||
| tests.cpp:273:2:273:8 | call to sprintf | This 'call to sprintf' operation requires 9 bytes but the destination is only 8 bytes. |
|
||||
| tests.cpp:308:3:308:9 | call to sprintf | This 'call to sprintf' operation requires 9 bytes but the destination is only 8 bytes. |
|
||||
| tests.cpp:315:2:315:8 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 4 bytes. |
|
||||
| tests.cpp:316:2:316:8 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 4 bytes. |
|
||||
| tests.cpp:321:2:321:8 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 4 bytes. |
|
||||
| tests.cpp:324:3:324:9 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 4 bytes. |
|
||||
| tests.cpp:327:2:327:8 | call to sprintf | This 'call to sprintf' operation requires 12 bytes but the destination is only 4 bytes. |
|
||||
| tests.cpp:329:3:329:9 | call to sprintf | This 'call to sprintf' operation requires 12 bytes but the destination is only 4 bytes. |
|
||||
| tests.cpp:341:2:341:8 | call to sprintf | This 'call to sprintf' operation requires 3 bytes but the destination is only 2 bytes. |
|
||||
| tests.cpp:343:2:343:8 | call to sprintf | This 'call to sprintf' operation requires 3 bytes but the destination is only 2 bytes. |
|
||||
| tests.cpp:345:2:345:8 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 2 bytes. |
|
||||
| tests.cpp:347:2:347:8 | call to sprintf | This 'call to sprintf' operation requires 3 bytes but the destination is only 2 bytes. |
|
||||
| tests.cpp:350:2:350:8 | call to sprintf | This 'call to sprintf' operation requires 4 bytes but the destination is only 3 bytes. |
|
||||
| tests.cpp:354:2:354:8 | call to sprintf | This 'call to sprintf' operation requires 4 bytes but the destination is only 3 bytes. |
|
||||
| tests.cpp:358:2:358:8 | call to sprintf | This 'call to sprintf' operation requires 4 bytes but the destination is only 3 bytes. |
|
||||
| tests.cpp:363:2:363:8 | call to sprintf | This 'call to sprintf' operation requires 5 bytes but the destination is only 4 bytes. |
|
||||
|
||||
@@ -0,0 +1,11 @@
|
||||
| tests.cpp:272:2:272:8 | call to sprintf | This 'call to sprintf' operation requires 9 bytes but the destination is only 8 bytes. |
|
||||
| tests.cpp:273:2:273:8 | call to sprintf | This 'call to sprintf' operation requires 9 bytes but the destination is only 8 bytes. |
|
||||
| tests.cpp:308:3:308:9 | call to sprintf | This 'call to sprintf' operation requires 9 bytes but the destination is only 8 bytes. |
|
||||
| tests.cpp:321:2:321:8 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 4 bytes. |
|
||||
| tests.cpp:324:3:324:9 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 4 bytes. |
|
||||
| tests.cpp:341:2:341:8 | call to sprintf | This 'call to sprintf' operation requires 3 bytes but the destination is only 2 bytes. |
|
||||
| tests.cpp:343:2:343:8 | call to sprintf | This 'call to sprintf' operation requires 3 bytes but the destination is only 2 bytes. |
|
||||
| tests.cpp:345:2:345:8 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 2 bytes. |
|
||||
| tests.cpp:347:2:347:8 | call to sprintf | This 'call to sprintf' operation requires 3 bytes but the destination is only 2 bytes. |
|
||||
| tests.cpp:354:2:354:8 | call to sprintf | This 'call to sprintf' operation requires 4 bytes but the destination is only 3 bytes. |
|
||||
| tests.cpp:358:2:358:8 | call to sprintf | This 'call to sprintf' operation requires 4 bytes but the destination is only 3 bytes. |
|
||||
@@ -0,0 +1 @@
|
||||
Security/CWE/CWE-120/VeryLikelyOverrunWrite.ql
|
||||
Reference in New Issue
Block a user