diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-125/DangerousUseMbtowc.ql b/cpp/ql/src/experimental/Security/CWE/CWE-125/DangerousUseMbtowc.ql index 2b668025794..da995ef48cb 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-125/DangerousUseMbtowc.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-125/DangerousUseMbtowc.ql @@ -23,7 +23,7 @@ predicate exprMayBeString(Expr exp) { fctmp.getAnArgument().(VariableAccess).getTarget() = exp.(VariableAccess).getTarget() or globalValueNumber(fctmp.getAnArgument()) = globalValueNumber(exp) ) and - fctmp.getTarget().hasGlobalOrStdName(["strlen", "strcat", "strncat", "strcpy", "sptintf"]) + fctmp.getTarget().hasName(["strlen", "strcat", "strncat", "strcpy", "sptintf", "printf"]) ) or exists(AssignExpr astmp | @@ -62,48 +62,176 @@ predicate argMacro(Expr exp) { ) } -from FunctionCall fc, string msg -where - exists(Loop lptmp | lptmp = fc.getEnclosingStmt().getParentStmt*()) and - fc.getTarget().hasGlobalOrStdName(["mbtowc", "mbrtowc"]) and - not fc.getArgument(0).isConstant() and - not fc.getArgument(1).isConstant() and - ( - exprMayBeString(fc.getArgument(1)) and - argConstOrSizeof(fc.getArgument(2)) and - fc.getArgument(2).getValue().toInt() < 5 and - not argMacro(fc.getArgument(2)) and - msg = "Size can be less than maximum character length, use macro MB_CUR_MAX." - or - not exprMayBeString(fc.getArgument(1)) and +/** Holds if erroneous situations of using functions `mbtowc` and `mbrtowc` are detected. */ +predicate findUseCharacterConversion(Expr exp, string msg) { + exists(FunctionCall fc | + fc = exp and ( - argConstOrSizeof(fc.getArgument(2)) - or - argMacro(fc.getArgument(2)) - or - exists(DecrementOperation dotmp | - globalValueNumber(dotmp.getAnOperand()) = globalValueNumber(fc.getArgument(2)) and - not exists(AssignSubExpr aetmp | + exists(Loop lptmp | lptmp = fc.getEnclosingStmt().getParentStmt*()) and + fc.getTarget().hasName(["mbtowc", "mbrtowc"]) and + not fc.getArgument(0).isConstant() and + not fc.getArgument(1).isConstant() and + ( + exprMayBeString(fc.getArgument(1)) and + argConstOrSizeof(fc.getArgument(2)) and + fc.getArgument(2).getValue().toInt() < 5 and + not argMacro(fc.getArgument(2)) and + msg = "Size can be less than maximum character length, use macro MB_CUR_MAX." + or + not exprMayBeString(fc.getArgument(1)) and + ( + argConstOrSizeof(fc.getArgument(2)) + or + argMacro(fc.getArgument(2)) + or + exists(DecrementOperation dotmp | + globalValueNumber(dotmp.getAnOperand()) = globalValueNumber(fc.getArgument(2)) and + not exists(AssignSubExpr aetmp | + ( + aetmp.getLValue().(VariableAccess).getTarget() = + fc.getArgument(2).(VariableAccess).getTarget() or + globalValueNumber(aetmp.getLValue()) = globalValueNumber(fc.getArgument(2)) + ) and + globalValueNumber(aetmp.getRValue()) = globalValueNumber(fc) + ) + ) + ) and + msg = + "Access beyond the allocated memory is possible, the length can change without changing the pointer." + or + exists(AssignPointerAddExpr aetmp | ( aetmp.getLValue().(VariableAccess).getTarget() = - fc.getArgument(2).(VariableAccess).getTarget() or - globalValueNumber(aetmp.getLValue()) = globalValueNumber(fc.getArgument(2)) + fc.getArgument(0).(VariableAccess).getTarget() or + globalValueNumber(aetmp.getLValue()) = globalValueNumber(fc.getArgument(0)) ) and globalValueNumber(aetmp.getRValue()) = globalValueNumber(fc) - ) + ) and + msg = "Maybe you're using the function's return value incorrectly." ) - ) and - msg = - "Access beyond the allocated memory is possible, the length can change without changing the pointer." - or - exists(AssignPointerAddExpr aetmp | - ( - aetmp.getLValue().(VariableAccess).getTarget() = - fc.getArgument(0).(VariableAccess).getTarget() or - globalValueNumber(aetmp.getLValue()) = globalValueNumber(fc.getArgument(0)) - ) and - globalValueNumber(aetmp.getRValue()) = globalValueNumber(fc) - ) and - msg = "Maybe you're using the function's return value incorrectly." + ) ) -select fc, msg +} + +/** Holds if detecting erroneous situations of working with multibyte characters. */ +predicate findUseMultibyteCharacter(Expr exp, string msg) { + exists(ArrayType arrayType, ArrayExpr arrayExpr | + arrayExpr = exp and + arrayExpr.getArrayBase().(VariableAccess).getTarget().getADeclarationEntry().getType() = + arrayType and + ( + exists(AssignExpr assZero, SizeofExprOperator sizeofArray, Expr oneValue | + oneValue.getValue() = "1" and + sizeofArray.getExprOperand().getType() = arrayType and + assZero.getLValue() = arrayExpr and + arrayExpr.getArrayOffset().(SubExpr).hasOperands(sizeofArray, oneValue) and + assZero.getRValue().getValue() = "0" + ) and + arrayType.getArraySize() != arrayType.getByteSize() and + msg = + "The size of the array element is greater than one byte, so the offset will point outside the array." + or + exists(FunctionCall mbFunction | + ( + mbFunction.getTarget().getName().matches("_mbs%") or + mbFunction.getTarget().getName().matches("mbs%") or + mbFunction.getTarget().getName().matches("_mbc%") or + mbFunction.getTarget().getName().matches("mbc%") + ) and + mbFunction.getAnArgument().(VariableAccess).getTarget().getADeclarationEntry().getType() = + arrayType + ) and + exists(Loop loop, SizeofExprOperator sizeofArray, AssignExpr assignExpr | + arrayExpr.getEnclosingStmt().getParentStmt*() = loop and + sizeofArray.getExprOperand().getType() = arrayType and + assignExpr.getLValue() = arrayExpr and + loop.getCondition().(LTExpr).getLeftOperand().(VariableAccess).getTarget() = + arrayExpr.getArrayOffset().getAChild*().(VariableAccess).getTarget() and + loop.getCondition().(LTExpr).getRightOperand() = sizeofArray + ) and + msg = + "This buffer may contain multibyte characters, so attempting to copy may result in part of the last character being lost." + ) + ) + or + exists(FunctionCall mbccpy, Loop loop, SizeofExprOperator sizeofOp | + mbccpy.getTarget().hasName("_mbccpy") and + mbccpy.getArgument(0) = exp and + exp.getEnclosingStmt().getParentStmt*() = loop and + sizeofOp.getExprOperand().getType() = + exp.getAChild*().(VariableAccess).getTarget().getADeclarationEntry().getType() and + loop.getCondition().(LTExpr).getLeftOperand().(VariableAccess).getTarget() = + exp.getAChild*().(VariableAccess).getTarget() and + loop.getCondition().(LTExpr).getRightOperand() = sizeofOp and + msg = + "This buffer may contain multibyte characters, so an attempt to copy may result in an overflow." + ) +} + +/** Holds if erroneous situations of using functions `MultiByteToWideChar` and `WideCharToMultiByte` or `mbstowcs` and `_mbstowcs_l` and `mbsrtowcs` are detected. */ +predicate findUseStringConversion( + Expr exp, string msg, int posBufSrc, int posBufDst, int posSizeDst, string nameCalls +) { + exists(FunctionCall fc | + fc = exp and + posBufSrc in [0 .. fc.getNumberOfArguments() - 1] and + posSizeDst in [0 .. fc.getNumberOfArguments() - 1] and + ( + fc.getTarget().hasName(nameCalls) and + ( + globalValueNumber(fc.getArgument(posBufDst)) = globalValueNumber(fc.getArgument(posBufSrc)) and + msg = + "According to the definition of the functions, if the source buffer and the destination buffer are the same, undefined behavior is possible." + or + exists(ArrayType arrayDst | + fc.getArgument(posBufDst).(VariableAccess).getTarget().getADeclarationEntry().getType() = + arrayDst and + fc.getArgument(posSizeDst).getValue().toInt() >= arrayDst.getArraySize() and + not exists(AssignExpr assZero | + assZero.getLValue().(ArrayExpr).getArrayBase().(VariableAccess).getTarget() = + fc.getArgument(posBufDst).(VariableAccess).getTarget() and + assZero.getRValue().getValue() = "0" + ) and + not exists(Expr someExp, FunctionCall checkSize | + checkSize.getASuccessor*() = fc and + checkSize.getTarget().hasName(nameCalls) and + checkSize.getArgument(posSizeDst).getValue() = "0" and + globalValueNumber(checkSize) = globalValueNumber(someExp) and + someExp.getEnclosingStmt().getParentStmt*() instanceof IfStmt + ) and + exprMayBeString(fc.getArgument(posBufDst)) and + msg = + "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." + ) + or + exists(FunctionCall allocMem | + allocMem.getTarget().hasName(["calloc", "malloc"]) and + globalValueNumber(fc.getArgument(posBufDst)) = globalValueNumber(allocMem) and + ( + allocMem.getArgument(allocMem.getNumberOfArguments() - 1).getValue() = "1" or + not exists(SizeofOperator sizeofOperator | + globalValueNumber(allocMem + .getArgument(allocMem.getNumberOfArguments() - 1) + .getAChild*()) = globalValueNumber(sizeofOperator) + ) + ) and + msg = + "The buffer destination has a type other than char, you need to take this into account when allocating memory." + ) + or + fc.getArgument(posBufDst).getValue() = "0" and + fc.getArgument(posSizeDst).getValue() != "0" and + msg = + "If the destination buffer is NULL and its size is not 0, then undefined behavior is possible." + ) + ) + ) +} + +from Expr exp, string msg +where + findUseCharacterConversion(exp, msg) or + findUseMultibyteCharacter(exp, msg) or + findUseStringConversion(exp, msg, 1, 0, 2, ["mbstowcs", "_mbstowcs_l", "mbsrtowcs"]) or + findUseStringConversion(exp, msg, 2, 4, 5, ["MultiByteToWideChar", "WideCharToMultiByte"]) +select exp, msg + exp.getEnclosingFunction().getName()