create new branchihsinme-patch-87 in fork

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

@@ -0,0 +1,6 @@
+
+...
+mbtowc(&wc, ptr, 4)); // BAD:we can get unpredictable results
+...
+mbtowc(&wc, ptr, MB_LEN_MAX); // GOOD
+...

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

@@ -0,0 +1,23 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p> Using function mbtowc with an invalid length argument can result in an out-of-bounds access error or unexpected result. If you are sure you are working with a null-terminated string, use the length macros, if not, use the correctly computed length.</p>
+
+</overview>
+
+<example>
+<p>The following example shows the erroneous and corrected method of using function mbtowc.</p>
+<sample src="DangerousUseMbtowc.cpp" />
+
+</example>
+<references>
+
+<li>
+  CERT Coding Standard:
+  <a href="https://wiki.sei.cmu.edu/confluence/display/c/ARR30-C.+Do+not+form+or+use+out-of-bounds+pointers+or+array+subscripts">ARR30-C. Do not form or use out-of-bounds pointers or array subscripts - SEI CERT C Coding Standard - Confluence</a>.
+</li>
+
+</references>
+</qhelp>

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

@@ -0,0 +1,106 @@
+/**
+ * @name Dangerous use mbtowc.
+ * @description Using function mbtowc with an invalid length argument can result in an out-of-bounds access error or unexpected result.
+ * @kind problem
+ * @id cpp/dangerous-use-mbtowc
+ * @problem.severity warning
+ * @precision medium
+ * @tags correctness
+ *       security
+ *       external/cwe/cwe-125
+ */
+
+import cpp
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+
+/** Holds if there are indications that the variable is treated as a string. */
+predicate exprMayBeString(Expr exp) {
+  (
+    exists(StringLiteral sl | globalValueNumber(exp) = globalValueNumber(sl))
+    or
+    exists(FunctionCall fctmp |
+      globalValueNumber(fctmp.getAnArgument()) = globalValueNumber(exp) and
+      fctmp.getTarget().hasGlobalOrStdName(["strlen", "strcat", "strncat", "strcpy", "sptintf"])
+    )
+    or
+    exists(AssignExpr astmp |
+      astmp.getRValue().getValue() = "0" and
+      astmp.getLValue().(ArrayExpr).getArrayBase().(VariableAccess).getTarget() =
+        exp.(VariableAccess).getTarget()
+    )
+    or
+    exists(ComparisonOperation cotmp, Expr exptmp1, Expr exptmp2 |
+      exptmp1.getValue() = "0" and
+      (
+        exptmp2.(PointerDereferenceExpr).getOperand().(VariableAccess).getTarget() =
+          exp.(VariableAccess).getTarget() or
+        exptmp2.(ArrayExpr).getArrayBase().(VariableAccess).getTarget() =
+          exp.getAChild().(VariableAccess).getTarget()
+      ) and
+      cotmp.hasOperands(exptmp1, exptmp2)
+    )
+  )
+}
+
+/** Holds if expression is constant or operator call `sizeof`. */
+predicate argConstOrSizeof(Expr exp) {
+  exp.getValue().toInt() > 1 or
+  exp.(SizeofTypeOperator).getTypeOperand().getSize() > 1
+}
+
+/** Holds if expression is macro. */
+predicate argMacro(Expr exp) {
+  exists(MacroInvocation matmp |
+    exp = matmp.getExpr() and
+    (
+      matmp.getMacroName() = "MB_LEN_MAX" or
+      matmp.getMacroName() = "MB_CUR_MAX"
+    )
+  )
+}
+
+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
+    (
+      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(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

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-125/semmle/tests/DangerousUseMbtowc.expected

@@ -0,0 +1,8 @@
+| test.cpp:61:27:61:32 | call to mbtowc | Size can be less than maximum character length, use macro MB_CUR_MAX. |
+| test.cpp:70:27:70:32 | call to mbtowc | Size can be less than maximum character length, use macro MB_CUR_MAX. |
+| test.cpp:98:11:98:16 | call to mbtowc | Access beyond the allocated memory is possible, the length can change without changing the pointer. |
+| test.cpp:114:11:114:16 | call to mbtowc | Access beyond the allocated memory is possible, the length can change without changing the pointer. |
+| test.cpp:130:11:130:16 | call to mbtowc | Access beyond the allocated memory is possible, the length can change without changing the pointer. |
+| test.cpp:147:11:147:16 | call to mbtowc | Access beyond the allocated memory is possible, the length can change without changing the pointer. |
+| test.cpp:169:11:169:16 | call to mbtowc | Access beyond the allocated memory is possible, the length can change without changing the pointer. |
+| test.cpp:185:11:185:16 | call to mbtowc | Maybe you're using the function's return value incorrectly. |

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-125/semmle/tests/DangerousUseMbtowc.qlref

@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-125/DangerousUseMbtowc.ql

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-125/semmle/tests/test.cpp

@@ -0,0 +1,194 @@
+typedef unsigned long size_t;
+#define MB_CUR_MAX 6
+#define MB_LEN_MAX 16
+int mbtowc(wchar_t *out, const char *in, size_t size);
+int wprintf (const wchar_t* format, ...);
+int strlen( const char * string );
+int checkErrors();
+
+void goodTest0()
+{
+  char * ptr = "123456789";
+  int ret;
+  int len;
+  len = 9;
+  for (wchar_t wc; (ret = mbtowc(&wc, ptr, len)) > 0; len-=ret) { // GOOD
+    wprintf(L"%lc", wc);
+  }
+}
+void goodTest1(const char* ptr)
+{
+  int ret;
+  int len;
+  len = strlen(ptr);
+  for (wchar_t wc; (ret = mbtowc(&wc, ptr, len)) > 0; len-=ret) { // GOOD
+    wprintf(L"%lc", wc);
+  }
+}
+void goodTest2(char* ptr)
+{
+  int ret;
+  ptr[10]=0;
+  int len = 9;
+  for (wchar_t wc; (ret = mbtowc(&wc, ptr, 16)) > 0; len-=ret) { // GOOD
+    wprintf(L"%lc", wc);
+  }
+}
+
+void goodTest3(const char* ptr)
+{
+  int ret;
+  int len;
+  len = strlen(ptr);
+  for (wchar_t wc; (ret = mbtowc(&wc, ptr, MB_CUR_MAX)) > 0; len-=ret) { // GOOD
+    wprintf(L"%lc", wc);
+  }
+}
+void goodTest4(const char* ptr)
+{
+  int ret;
+  int len;
+  len = strlen(ptr);
+  for (wchar_t wc; (ret = mbtowc(&wc, ptr, MB_LEN_MAX)) > 0; len-=ret) { // GOOD
+    wprintf(L"%lc", wc);
+  }
+}
+void badTest1(const char* ptr)
+{
+  int ret;
+  int len;
+  len = strlen(ptr);
+  for (wchar_t wc; (ret = mbtowc(&wc, ptr, 4)) > 0; len-=ret) { // BAD:we can get unpredictable results
+    wprintf(L"%lc", wc);
+  }
+}
+void badTest2(const char* ptr)
+{
+  int ret;
+  int len;
+  len = strlen(ptr);
+  for (wchar_t wc; (ret = mbtowc(&wc, ptr, sizeof(wchar_t))) > 0; len-=ret) { // BAD:we can get unpredictable results
+    wprintf(L"%lc", wc);
+  }
+}
+
+void goodTest5(const char* ptr,wchar_t *wc,int wc_len)
+{
+  int ret;
+  int len;
+  len = wc_len;
+  while (*ptr && len > 0) {
+    ret = mbtowc(wc, ptr, len); // GOOD
+    if (ret <0)
+      break;
+    if (ret == 0 || ret > len)
+      break;
+    len-=ret;
+    ptr+=ret;
+    wc++;
+  }
+}
+
+void badTest3(const char* ptr,wchar_t *wc,int wc_len)
+{
+  int ret;
+  int len;
+  len = wc_len;
+  while (*ptr && len > 0) {
+    ret = mbtowc(wc, ptr, MB_CUR_MAX); // BAD
+    if (ret <0)
+      break;
+    if (ret == 0 || ret > len)
+      break;
+    len-=ret;
+    ptr+=ret;
+    wc++;
+  }
+}
+void badTest4(const char* ptr,wchar_t *wc,int wc_len)
+{
+  int ret;
+  int len;
+  len = wc_len;
+  while (*ptr && len > 0) {
+    ret = mbtowc(wc, ptr, 16); // BAD
+    if (ret <0)
+      break;
+    if (ret == 0 || ret > len)
+      break;
+    len-=ret;
+    ptr+=ret;
+    wc++;
+  }
+}
+void badTest5(const char* ptr,wchar_t *wc,int wc_len)
+{
+  int ret;
+  int len;
+  len = wc_len;
+  while (*ptr && len > 0) {
+    ret = mbtowc(wc, ptr, sizeof(wchar_t)); // BAD
+    if (ret <0)
+      break;
+    if (ret == 0 || ret > len)
+      break;
+    len-=ret;
+    ptr+=ret;
+    wc++;
+  }
+}
+
+void badTest6(const char* ptr,wchar_t *wc,int wc_len)
+{
+  int ret;
+  int len;
+  len = wc_len;
+  while (*ptr && wc_len > 0) {
+    ret = mbtowc(wc, ptr, wc_len); // BAD
+    if (ret <0)
+      if (checkErrors()) {
+        ++ptr;
+        --len;
+        continue;
+      } else
+        break;
+    if (ret == 0 || ret > len)
+      break;
+    wc_len--;
+    len-=ret;
+    wc++;
+    ptr+=ret;
+  }
+}
+void badTest7(const char* ptr,wchar_t *wc,int wc_len)
+{
+  int ret;
+  int len;
+  len = wc_len;
+  while (*ptr && wc_len > 0) {
+    ret = mbtowc(wc, ptr, len); // BAD
+    if (ret <0)
+        break;
+    if (ret == 0 || ret > len)
+      break;
+    len--;
+    wc++;
+    ptr+=ret;
+  }
+}
+void badTest8(const char* ptr,wchar_t *wc)
+{
+  int ret;
+  int len;
+  len = strlen(ptr);
+  while (*ptr && len > 0) {
+    ret = mbtowc(wc, ptr, len); // BAD
+    if (ret <0)
+      break;
+    if (ret == 0 || ret > len)
+      break;
+    len-=ret;
+    ptr++;
+    wc+=ret;
+  }
+}