Skip to content

Commit f53adca

Browse files
ihsinmeihsinme
authored
Update DangerousUseMbtowc.ql
1 parent 9d12f1b commit f53adca

File tree

1 file changed

+165
-37
lines changed

1 file changed

+165
-37
lines changed

‎cpp/ql/src/experimental/Security/CWE/CWE-125/DangerousUseMbtowc.ql

Lines changed: 165 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ predicate exprMayBeString(Expr exp) {
2323
fctmp.getAnArgument().(VariableAccess).getTarget() = exp.(VariableAccess).getTarget() or
2424
globalValueNumber(fctmp.getAnArgument()) = globalValueNumber(exp)
2525
) and
26-
fctmp.getTarget().hasGlobalOrStdName(["strlen", "strcat", "strncat", "strcpy", "sptintf"])
26+
fctmp.getTarget().hasName(["strlen", "strcat", "strncat", "strcpy", "sptintf", "printf"])
2727
)
2828
or
2929
exists(AssignExpr astmp |
@@ -62,48 +62,176 @@ predicate argMacro(Expr exp) {
6262
)
6363
}
6464

65-
from FunctionCall fc, string msg
66-
where
67-
exists(Loop lptmp | lptmp = fc.getEnclosingStmt().getParentStmt*()) and
68-
fc.getTarget().hasGlobalOrStdName(["mbtowc", "mbrtowc"]) and
69-
not fc.getArgument(0).isConstant() and
70-
not fc.getArgument(1).isConstant() and
71-
(
72-
exprMayBeString(fc.getArgument(1)) and
73-
argConstOrSizeof(fc.getArgument(2)) and
74-
fc.getArgument(2).getValue().toInt() < 5 and
75-
not argMacro(fc.getArgument(2)) and
76-
msg = "Size can be less than maximum character length, use macro MB_CUR_MAX."
77-
or
78-
not exprMayBeString(fc.getArgument(1)) and
65+
/** Holds if erroneous situations of using functions `mbtowc` and `mbrtowc` are detected. */
66+
predicate findUseCharacterConversion(Expr exp, string msg) {
67+
exists(FunctionCall fc |
68+
fc = exp and
7969
(
80-
argConstOrSizeof(fc.getArgument(2))
81-
or
82-
argMacro(fc.getArgument(2))
83-
or
84-
exists(DecrementOperation dotmp |
85-
globalValueNumber(dotmp.getAnOperand()) = globalValueNumber(fc.getArgument(2)) and
86-
not exists(AssignSubExpr aetmp |
70+
exists(Loop lptmp | lptmp = fc.getEnclosingStmt().getParentStmt*()) and
71+
fc.getTarget().hasName(["mbtowc", "mbrtowc"]) and
72+
not fc.getArgument(0).isConstant() and
73+
not fc.getArgument(1).isConstant() and
74+
(
75+
exprMayBeString(fc.getArgument(1)) and
76+
argConstOrSizeof(fc.getArgument(2)) and
77+
fc.getArgument(2).getValue().toInt() < 5 and
78+
not argMacro(fc.getArgument(2)) and
79+
msg = "Size can be less than maximum character length, use macro MB_CUR_MAX."
80+
or
81+
not exprMayBeString(fc.getArgument(1)) and
82+
(
83+
argConstOrSizeof(fc.getArgument(2))
84+
or
85+
argMacro(fc.getArgument(2))
86+
or
87+
exists(DecrementOperation dotmp |
88+
globalValueNumber(dotmp.getAnOperand()) = globalValueNumber(fc.getArgument(2)) and
89+
not exists(AssignSubExpr aetmp |
90+
(
91+
aetmp.getLValue().(VariableAccess).getTarget() =
92+
fc.getArgument(2).(VariableAccess).getTarget() or
93+
globalValueNumber(aetmp.getLValue()) = globalValueNumber(fc.getArgument(2))
94+
) and
95+
globalValueNumber(aetmp.getRValue()) = globalValueNumber(fc)
96+
)
97+
)
98+
) and
99+
msg =
100+
"Access beyond the allocated memory is possible, the length can change without changing the pointer."
101+
or
102+
exists(AssignPointerAddExpr aetmp |
87103
(
88104
aetmp.getLValue().(VariableAccess).getTarget() =
89-
fc.getArgument(2).(VariableAccess).getTarget() or
90-
globalValueNumber(aetmp.getLValue()) = globalValueNumber(fc.getArgument(2))
105+
fc.getArgument(0).(VariableAccess).getTarget() or
106+
globalValueNumber(aetmp.getLValue()) = globalValueNumber(fc.getArgument(0))
91107
) and
92108
globalValueNumber(aetmp.getRValue()) = globalValueNumber(fc)
93-
)
109+
) and
110+
msg = "Maybe you're using the function's return value incorrectly."
94111
)
95-
) and
112+
)
113+
)
114+
}
115+
116+
/** Holds if detecting erroneous situations of working with multibyte characters. */
117+
predicate findUseMultibyteCharacter(Expr exp, string msg) {
118+
exists(ArrayType arrayType, ArrayExpr arrayExpr |
119+
arrayExpr = exp and
120+
arrayExpr.getArrayBase().(VariableAccess).getTarget().getADeclarationEntry().getType() =
121+
arrayType and
122+
(
123+
exists(AssignExpr assZero, SizeofExprOperator sizeofArray, Expr oneValue |
124+
oneValue.getValue() = "1" and
125+
sizeofArray.getExprOperand().getType() = arrayType and
126+
assZero.getLValue() = arrayExpr and
127+
arrayExpr.getArrayOffset().(SubExpr).hasOperands(sizeofArray, oneValue) and
128+
assZero.getRValue().getValue() = "0"
129+
) and
130+
arrayType.getArraySize() != arrayType.getByteSize() and
131+
msg =
132+
"The size of the array element is greater than one byte, so the offset will point outside the array."
133+
or
134+
exists(FunctionCall mbFunction |
135+
(
136+
mbFunction.getTarget().getName().matches("_mbs%") or
137+
mbFunction.getTarget().getName().matches("mbs%") or
138+
mbFunction.getTarget().getName().matches("_mbc%") or
139+
mbFunction.getTarget().getName().matches("mbc%")
140+
) and
141+
mbFunction.getAnArgument().(VariableAccess).getTarget().getADeclarationEntry().getType() =
142+
arrayType
143+
) and
144+
exists(Loop loop, SizeofExprOperator sizeofArray, AssignExpr assignExpr |
145+
arrayExpr.getEnclosingStmt().getParentStmt*() = loop and
146+
sizeofArray.getExprOperand().getType() = arrayType and
147+
assignExpr.getLValue() = arrayExpr and
148+
loop.getCondition().(LTExpr).getLeftOperand().(VariableAccess).getTarget() =
149+
arrayExpr.getArrayOffset().getAChild*().(VariableAccess).getTarget() and
150+
loop.getCondition().(LTExpr).getRightOperand() = sizeofArray
151+
) and
152+
msg =
153+
"This buffer may contain multibyte characters, so attempting to copy may result in part of the last character being lost."
154+
)
155+
)
156+
or
157+
exists(FunctionCall mbccpy, Loop loop, SizeofExprOperator sizeofOp |
158+
mbccpy.getTarget().hasName("_mbccpy") and
159+
mbccpy.getArgument(0) = exp and
160+
exp.getEnclosingStmt().getParentStmt*() = loop and
161+
sizeofOp.getExprOperand().getType() =
162+
exp.getAChild*().(VariableAccess).getTarget().getADeclarationEntry().getType() and
163+
loop.getCondition().(LTExpr).getLeftOperand().(VariableAccess).getTarget() =
164+
exp.getAChild*().(VariableAccess).getTarget() and
165+
loop.getCondition().(LTExpr).getRightOperand() = sizeofOp and
96166
msg =
97-
"Access beyond the allocated memory is possible, the length can change without changing the pointer."
98-
or
99-
exists(AssignPointerAddExpr aetmp |
167+
"This buffer may contain multibyte characters, so an attempt to copy may result in an overflow."
168+
)
169+
}
170+
171+
/** Holds if erroneous situations of using functions `MultiByteToWideChar` and `WideCharToMultiByte` or `mbstowcs` and `_mbstowcs_l` and `mbsrtowcs` are detected. */
172+
predicate findUseStringConversion(
173+
Expr exp, string msg, int posBufSrc, int posBufDst, int posSizeDst, string nameCalls
174+
) {
175+
exists(FunctionCall fc |
176+
fc = exp and
177+
posBufSrc in [0 .. fc.getNumberOfArguments() - 1] and
178+
posSizeDst in [0 .. fc.getNumberOfArguments() - 1] and
179+
(
180+
fc.getTarget().hasName(nameCalls) and
100181
(
101-
aetmp.getLValue().(VariableAccess).getTarget() =
102-
fc.getArgument(0).(VariableAccess).getTarget() or
103-
globalValueNumber(aetmp.getLValue()) = globalValueNumber(fc.getArgument(0))
104-
) and
105-
globalValueNumber(aetmp.getRValue()) = globalValueNumber(fc)
106-
) and
107-
msg = "Maybe you're using the function's return value incorrectly."
182+
globalValueNumber(fc.getArgument(posBufDst)) = globalValueNumber(fc.getArgument(posBufSrc)) and
183+
msg =
184+
"According to the definition of the functions, if the source buffer and the destination buffer are the same, undefined behavior is possible."
185+
or
186+
exists(ArrayType arrayDst |
187+
fc.getArgument(posBufDst).(VariableAccess).getTarget().getADeclarationEntry().getType() =
188+
arrayDst and
189+
fc.getArgument(posSizeDst).getValue().toInt() >= arrayDst.getArraySize() and
190+
not exists(AssignExpr assZero |
191+
assZero.getLValue().(ArrayExpr).getArrayBase().(VariableAccess).getTarget() =
192+
fc.getArgument(posBufDst).(VariableAccess).getTarget() and
193+
assZero.getRValue().getValue() = "0"
194+
) and
195+
not exists(Expr someExp, FunctionCall checkSize |
196+
checkSize.getASuccessor*() = fc and
197+
checkSize.getTarget().hasName(nameCalls) and
198+
checkSize.getArgument(posSizeDst).getValue() = "0" and
199+
globalValueNumber(checkSize) = globalValueNumber(someExp) and
200+
someExp.getEnclosingStmt().getParentStmt*() instanceof IfStmt
201+
) and
202+
exprMayBeString(fc.getArgument(posBufDst)) and
203+
msg =
204+
"According to the definition of the functions, it is not guaranteed to write a null character at the end of the string, so access beyond the bounds of the destination buffer is possible."
205+
)
206+
or
207+
exists(FunctionCall allocMem |
208+
allocMem.getTarget().hasName(["calloc", "malloc"]) and
209+
globalValueNumber(fc.getArgument(posBufDst)) = globalValueNumber(allocMem) and
210+
(
211+
allocMem.getArgument(allocMem.getNumberOfArguments() - 1).getValue() = "1" or
212+
not exists(SizeofOperator sizeofOperator |
213+
globalValueNumber(allocMem
214+
.getArgument(allocMem.getNumberOfArguments() - 1)
215+
.getAChild*()) = globalValueNumber(sizeofOperator)
216+
)
217+
) and
218+
msg =
219+
"The buffer destination has a type other than char, you need to take this into account when allocating memory."
220+
)
221+
or
222+
fc.getArgument(posBufDst).getValue() = "0" and
223+
fc.getArgument(posSizeDst).getValue() != "0" and
224+
msg =
225+
"If the destination buffer is NULL and its size is not 0, then undefined behavior is possible."
226+
)
227+
)
108228
)
109-
select fc, msg
229+
}
230+
231+
from Expr exp, string msg
232+
where
233+
findUseCharacterConversion(exp, msg) or
234+
findUseMultibyteCharacter(exp, msg) or
235+
findUseStringConversion(exp, msg, 1, 0, 2, ["mbstowcs", "_mbstowcs_l", "mbsrtowcs"]) or
236+
findUseStringConversion(exp, msg, 2, 4, 5, ["MultiByteToWideChar", "WideCharToMultiByte"])
237+
select exp, msg + exp.getEnclosingFunction().getName()

0 commit comments

Comments
 (0)