Skip to content

Commit 598f283

Browse files
authored
C++: add reason to buffer write estimations
1 parent 7ff2ee6 commit 598f283

File tree

5 files changed

+174
-111
lines changed

5 files changed

+174
-111
lines changed

cpp/ql/lib/semmle/code/cpp/commons/Printf.qll

Lines changed: 104 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,30 @@ import semmle.code.cpp.models.interfaces.FormattingFunction
99
private import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
1010
private import semmle.code.cpp.rangeanalysis.RangeAnalysisUtils
1111

12+
private newtype TBufferWriteEstimationReason =
13+
TTypeBoundsAnalysis() or
14+
TValueFlowAnalysis()
15+
16+
class BufferWriteEstimationReason extends TBufferWriteEstimationReason {
17+
BufferWriteEstimationReason() {
18+
this = TTypeBoundsAnalysis() or
19+
this = TValueFlowAnalysis()
20+
}
21+
22+
string toString() {
23+
this = TTypeBoundsAnalysis() and result = "based on type bounds" or
24+
this = TValueFlowAnalysis() and result = "based on flow analysis of value bounds"
25+
}
26+
27+
BufferWriteEstimationReason combineWith(BufferWriteEstimationReason other) {
28+
(this = TTypeBoundsAnalysis() or other = TTypeBoundsAnalysis()) and result = TTypeBoundsAnalysis() or
29+
(this = TValueFlowAnalysis() and other = TValueFlowAnalysis()) and result = TValueFlowAnalysis()
30+
}
31+
}
32+
33+
BufferWriteEstimationReason typeBoundsAnalysis() { result = TTypeBoundsAnalysis() }
34+
BufferWriteEstimationReason valueFlowAnalysis() { result = TValueFlowAnalysis() }
35+
1236
class PrintfFormatAttribute extends FormatAttribute {
1337
PrintfFormatAttribute() { this.getArchetype() = ["printf", "__printf__"] }
1438
}
@@ -990,7 +1014,7 @@ class FormatLiteral extends Literal {
9901014
* conversion specifier of this format string; has no result if this cannot
9911015
* be determined.
9921016
*/
993-
int getMaxConvertedLength(int n) {
1017+
int getMaxConvertedLength(int n, BufferWriteEstimationReason reason) {
9941018
exists(int len |
9951019
(
9961020
(
@@ -1002,10 +1026,10 @@ class FormatLiteral extends Literal {
10021026
) and
10031027
(
10041028
this.getConversionChar(n) = "%" and
1005-
len = 1
1029+
len = 1 and reason = TValueFlowAnalysis()
10061030
or
10071031
this.getConversionChar(n).toLowerCase() = "c" and
1008-
len = 1 // e.g. 'a'
1032+
len = 1 and reason = TValueFlowAnalysis() // e.g. 'a'
10091033
or
10101034
this.getConversionChar(n).toLowerCase() = "f" and
10111035
exists(int dot, int afterdot |
@@ -1019,7 +1043,7 @@ class FormatLiteral extends Literal {
10191043
afterdot = 6
10201044
) and
10211045
len = 1 + 309 + dot + afterdot
1022-
) // e.g. -1e308="-100000"...
1046+
) and reason = TTypeBoundsAnalysis() // e.g. -1e308="-100000"...
10231047
or
10241048
this.getConversionChar(n).toLowerCase() = "e" and
10251049
exists(int dot, int afterdot |
@@ -1033,7 +1057,7 @@ class FormatLiteral extends Literal {
10331057
afterdot = 6
10341058
) and
10351059
len = 1 + 1 + dot + afterdot + 1 + 1 + 3
1036-
) // -1e308="-1.000000e+308"
1060+
) and reason = TTypeBoundsAnalysis() // -1e308="-1.000000e+308"
10371061
or
10381062
this.getConversionChar(n).toLowerCase() = "g" and
10391063
exists(int dot, int afterdot |
@@ -1056,67 +1080,78 @@ class FormatLiteral extends Literal {
10561080
// (e.g. 123456, 0.000123456 are just OK)
10571081
// so case %f can be at most P characters + 4 zeroes, sign, dot = P + 6
10581082
len = (afterdot.maximum(1) + 6).maximum(1 + 1 + dot + afterdot + 1 + 1 + 3)
1059-
) // (e.g. "-1.59203e-319")
1083+
) and reason = TTypeBoundsAnalysis() // (e.g. "-1.59203e-319")
10601084
or
10611085
this.getConversionChar(n).toLowerCase() = ["d", "i"] and
10621086
// e.g. -2^31 = "-2147483648"
1063-
len =
1064-
min(float cand |
1087+
exists(float typeBasedBound, float valueBasedBound |
10651088
// The first case handles length sub-specifiers
10661089
// Subtract one in the exponent because one bit is for the sign.
10671090
// Add 1 to account for the possible sign in the output.
1068-
cand = 1 + lengthInBase10(2.pow(this.getIntegralDisplayType(n).getSize() * 8 - 1))
1069-
or
1091+
typeBasedBound = 1 + lengthInBase10(2.pow(this.getIntegralDisplayType(n).getSize() * 8 - 1)) and
10701092
// The second case uses range analysis to deduce a length that's shorter than the length
10711093
// of the number -2^31.
1072-
exists(Expr arg, float lower, float upper |
1094+
exists(Expr arg, float lower, float upper, float typeLower, float typeUpper |
10731095
arg = this.getUse().getConversionArgument(n) and
10741096
lower = lowerBound(arg.getFullyConverted()) and
1075-
upper = upperBound(arg.getFullyConverted())
1097+
upper = upperBound(arg.getFullyConverted()) and
1098+
typeLower = exprMinVal(arg.getFullyConverted()) and
1099+
typeUpper = exprMaxVal(arg.getFullyConverted())
10761100
|
1077-
cand =
1078-
max(int cand0 |
1101+
valueBasedBound =
1102+
max(int cand |
10791103
// Include the sign bit in the length if it can be negative
10801104
(
10811105
if lower < 0
1082-
then cand0 = 1 + lengthInBase10(lower.abs())
1083-
else cand0 = lengthInBase10(lower)
1106+
then cand = 1 + lengthInBase10(lower.abs())
1107+
else cand = lengthInBase10(lower)
10841108
)
10851109
or
10861110
(
10871111
if upper < 0
1088-
then cand0 = 1 + lengthInBase10(upper.abs())
1089-
else cand0 = lengthInBase10(upper)
1112+
then cand = 1 + lengthInBase10(upper.abs())
1113+
else cand = lengthInBase10(upper)
10901114
)
1091-
)
1092-
)
1093-
)
1115+
) and
1116+
(
1117+
if lower > typeLower or upper < typeUpper
1118+
then reason = TValueFlowAnalysis()
1119+
else reason = TTypeBoundsAnalysis()
1120+
)
1121+
) and
1122+
len = valueBasedBound.minimum(typeBasedBound)
1123+
)
10941124
or
10951125
this.getConversionChar(n).toLowerCase() = "u" and
10961126
// e.g. 2^32 - 1 = "4294967295"
1097-
len =
1098-
min(float cand |
1127+
exists(float typeBasedBound, float valueBasedBound |
10991128
// The first case handles length sub-specifiers
1100-
cand = 2.pow(this.getIntegralDisplayType(n).getSize() * 8)
1101-
or
1129+
typeBasedBound = lengthInBase10(2.pow(this.getIntegralDisplayType(n).getSize() * 8) - 1) and
11021130
// The second case uses range analysis to deduce a length that's shorter than
11031131
// the length of the number 2^31 - 1.
1104-
exists(Expr arg, float lower |
1132+
exists(Expr arg, float lower, float upper, float typeLower, float typeUpper |
11051133
arg = this.getUse().getConversionArgument(n) and
1106-
lower = lowerBound(arg.getFullyConverted())
1134+
lower = lowerBound(arg.getFullyConverted()) and
1135+
upper = upperBound(arg.getFullyConverted()) and
1136+
typeLower = exprMinVal(arg.getFullyConverted()) and
1137+
typeUpper = exprMaxVal(arg.getFullyConverted())
11071138
|
1108-
cand =
1109-
max(float cand0 |
1139+
valueBasedBound =
1140+
lengthInBase10(max(float cand |
11101141
// If lower can be negative we use `(unsigned)-1` as the candidate value.
11111142
lower < 0 and
1112-
cand0 = 2.pow(any(IntType t | t.isUnsigned()).getSize() * 8)
1143+
cand = 2.pow(any(IntType t | t.isUnsigned()).getSize() * 8)
11131144
or
1114-
cand0 = upperBound(arg.getFullyConverted())
1115-
)
1116-
)
1117-
|
1118-
lengthInBase10(cand)
1119-
)
1145+
cand = upper
1146+
)) and
1147+
(
1148+
if lower > typeLower or upper < typeUpper
1149+
then reason = TValueFlowAnalysis()
1150+
else reason = TTypeBoundsAnalysis()
1151+
)
1152+
) and
1153+
len = valueBasedBound.minimum(typeBasedBound)
1154+
)
11201155
or
11211156
this.getConversionChar(n).toLowerCase() = "x" and
11221157
// e.g. "12345678"
@@ -1135,7 +1170,7 @@ class FormatLiteral extends Literal {
11351170
(
11361171
if this.hasAlternateFlag(n) then len = 2 + baseLen else len = baseLen // "0x"
11371172
)
1138-
)
1173+
) and reason = TTypeBoundsAnalysis()
11391174
or
11401175
this.getConversionChar(n).toLowerCase() = "p" and
11411176
exists(PointerType ptrType, int baseLen |
@@ -1144,7 +1179,7 @@ class FormatLiteral extends Literal {
11441179
(
11451180
if this.hasAlternateFlag(n) then len = 2 + baseLen else len = baseLen // "0x"
11461181
)
1147-
)
1182+
) and reason = TValueFlowAnalysis()
11481183
or
11491184
this.getConversionChar(n).toLowerCase() = "o" and
11501185
// e.g. 2^32 - 1 = "37777777777"
@@ -1163,14 +1198,15 @@ class FormatLiteral extends Literal {
11631198
(
11641199
if this.hasAlternateFlag(n) then len = 1 + baseLen else len = baseLen // "0"
11651200
)
1166-
)
1201+
) and reason = TTypeBoundsAnalysis()
11671202
or
11681203
this.getConversionChar(n).toLowerCase() = "s" and
11691204
len =
11701205
min(int v |
11711206
v = this.getPrecision(n) or
11721207
v = this.getUse().getFormatArgument(n).(AnalysedString).getMaxLength() - 1 // (don't count null terminator)
1173-
)
1208+
) and
1209+
reason = TValueFlowAnalysis()
11741210
)
11751211
)
11761212
}
@@ -1182,10 +1218,10 @@ class FormatLiteral extends Literal {
11821218
* determining whether a buffer overflow is caused by long float to string
11831219
* conversions.
11841220
*/
1185-
int getMaxConvertedLengthLimited(int n) {
1221+
int getMaxConvertedLengthLimited(int n, BufferWriteEstimationReason reason) {
11861222
if this.getConversionChar(n).toLowerCase() = "f"
1187-
then result = this.getMaxConvertedLength(n).minimum(8)
1188-
else result = this.getMaxConvertedLength(n)
1223+
then result = this.getMaxConvertedLength(n, reason).minimum(8)
1224+
else result = this.getMaxConvertedLength(n, reason)
11891225
}
11901226

11911227
/**
@@ -1225,35 +1261,49 @@ class FormatLiteral extends Literal {
12251261
)
12261262
}
12271263

1228-
private int getMaxConvertedLengthAfter(int n) {
1264+
private int getMaxConvertedLengthAfter(int n, BufferWriteEstimationReason reason) {
12291265
if n = this.getNumConvSpec()
1230-
then result = this.getConstantSuffix().length() + 1
1266+
then result = this.getConstantSuffix().length() + 1 and reason = TValueFlowAnalysis()
12311267
else
1232-
result =
1233-
this.getConstantPart(n).length() + this.getMaxConvertedLength(n) +
1234-
this.getMaxConvertedLengthAfter(n + 1)
1268+
exists(BufferWriteEstimationReason headReason, BufferWriteEstimationReason tailReason |
1269+
result =
1270+
this.getConstantPart(n).length() + this.getMaxConvertedLength(n, headReason) +
1271+
this.getMaxConvertedLengthAfter(n + 1, tailReason) and
1272+
reason = headReason.combineWith(tailReason)
1273+
)
12351274
}
12361275

1237-
private int getMaxConvertedLengthAfterLimited(int n) {
1276+
private int getMaxConvertedLengthAfterLimited(int n, BufferWriteEstimationReason reason) {
12381277
if n = this.getNumConvSpec()
1239-
then result = this.getConstantSuffix().length() + 1
1278+
then result = this.getConstantSuffix().length() + 1 and reason = TValueFlowAnalysis()
12401279
else
1280+
exists(BufferWriteEstimationReason headReason, BufferWriteEstimationReason tailReason |
12411281
result =
1242-
this.getConstantPart(n).length() + this.getMaxConvertedLengthLimited(n) +
1243-
this.getMaxConvertedLengthAfterLimited(n + 1)
1282+
this.getConstantPart(n).length() + this.getMaxConvertedLengthLimited(n, headReason) +
1283+
this.getMaxConvertedLengthAfterLimited(n + 1, tailReason) and
1284+
reason = headReason.combineWith(tailReason)
1285+
)
12441286
}
12451287

12461288
/**
12471289
* Gets the maximum length of the string that can be produced by this format
12481290
* string. Has no result if this cannot be determined.
12491291
*/
1250-
int getMaxConvertedLength() { result = this.getMaxConvertedLengthAfter(0) }
1292+
int getMaxConvertedLength() { result = this.getMaxConvertedLengthAfter(0, _) }
12511293

12521294
/**
12531295
* Gets the maximum length of the string that can be produced by this format
12541296
* string, except that float to string conversions are assumed to be 8
12551297
* characters. This is helpful for determining whether a buffer overflow
12561298
* is caused by long float to string conversions.
12571299
*/
1258-
int getMaxConvertedLengthLimited() { result = this.getMaxConvertedLengthAfterLimited(0) }
1300+
int getMaxConvertedLengthLimited() { result = this.getMaxConvertedLengthAfterLimited(0, _) }
1301+
1302+
int getMaxConvertedLengthWithReason(BufferWriteEstimationReason reason) {
1303+
result = this.getMaxConvertedLengthAfter(0, reason)
1304+
}
1305+
1306+
int getMaxConvertedLengthLimitedWithReason(BufferWriteEstimationReason reason) {
1307+
result = this.getMaxConvertedLengthAfterLimited(0, reason)
1308+
}
12591309
}

0 commit comments

Comments
 (0)