Merge pull request #264 from raulgarciamsft/users/raulga/c6276

C++: incorrect string type conversion

Author: jbj

cpp/ql/src/Security/CWE/CWE-704/WcharCharConversion.cpp

@@ -0,0 +1,3 @@
+wchar_t* pSrc;
+
+pSrc = (wchar_t*)"a"; // casting a byte-string literal "a" to a wide-character string

cpp/ql/src/Security/CWE/CWE-704/WcharCharConversion.qhelp

@@ -0,0 +1,35 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+  <p>This rule indicates a potentially incorrect cast from an byte string (<code>char *</code>) to a wide-character string (<code>wchar_t *</code>).</p>
+  <p>This cast might yield strings that are not correctly terminated; including potential buffer overruns when using such strings with some dangerous APIs.</p>
+</overview>
+
+<recommendation>
+  <p>Do not explicitly cast byte strings to wide-character strings.</p>
+  <p>For string literals, prepend the literal string with the letter "L" to indicate that the string is a wide-character string (<code>wchar_t *</code>).</p>
+  <p>For converting a byte literal to a wide-character string literal, you would need to use the appropriate conversion function for the platform you are using. Please see the references section for options according to your platform.</p>
+</recommendation>
+
+<example>
+<p>In the following example, an byte string literal (<code>"a"</code>) is cast to a wide-character string.</p>
+<sample src="WcharCharConversion.cpp" />
+
+<p>To fix this issue, prepend the literal with the letter "L" (<code>L"a"</code>) to define it as a wide-character string.</p>
+</example>
+
+<references>
+  <li>
+    General resources:
+    <a href="https://en.cppreference.com/w/cpp/string/multibyte/mbstowcs">std::mbstowcs</a>
+  </li>
+  <li>
+    Microsoft specific resources:
+    <a href="https://docs.microsoft.com/en-us/windows/desktop/Intl/security-considerations--international-features">Security Considerations: International Features</a>
+  </li>
+</references>
+
+</qhelp>

cpp/ql/src/Security/CWE/CWE-704/WcharCharConversion.ql

@@ -0,0 +1,29 @@
+/**
+ * @name Cast from char* to wchar_t*
+ * @description Casting a byte string to a wide-character string is likely 
+ *              to yield a string that is incorrectly terminated or aligned. 
+ *              This can lead to undefined behavior, including buffer overruns.
+ * @kind problem
+ * @id cpp/incorrect-string-type-conversion
+ * @problem.severity error
+ * @precision high
+ * @tags security
+ *       external/cwe/cwe-704
+ *       external/microsoft/c/c6276
+ */
+import cpp
+
+class WideCharPointerType extends PointerType {
+  WideCharPointerType() { 
+    this.getBaseType() instanceof WideCharType 
+  }
+}
+
+from Expr e1, Cast e2
+where 
+  e2 = e1.getConversion() and
+  exists(WideCharPointerType w, CharPointerType c |
+    w = e2.getType().getUnspecifiedType().(PointerType) and
+    c = e1.getType().getUnspecifiedType().(PointerType)
+  )
+select e1, "Conversion from " + e1.getType().toString() + " to " +  e2.getType().toString() + ". Use of invalid string can lead to undefined behavior."

cpp/ql/test/query-tests/Security/CWE/CWE-704/WcharCharConversion.cpp

@@ -0,0 +1,34 @@
+#define NULL 0
+#define CONST               const
+typedef wchar_t WCHAR;    // wc,   16-bit UNICODE character
+typedef char CHAR;
+
+typedef WCHAR *LPWSTR;
+typedef CONST WCHAR *LPCWSTR;
+
+typedef CHAR *LPSTR;
+typedef CONST CHAR *LPCSTR;
+
+void fconstWChar(LPCWSTR p) {}
+void fWChar(LPWSTR p) {}
+
+void Test()
+{
+	char *lpChar = NULL;
+	wchar_t *lpWchar = NULL;
+	LPCSTR lpcstr = "b";
+
+	lpWchar = (LPWSTR)"a"; // BUG
+	lpWchar = (LPWSTR)lpcstr; // BUG
+
+	lpWchar = (wchar_t*)lpChar;	// BUG
+
+	fconstWChar((LPCWSTR)lpChar);	// BUG
+	fWChar((LPWSTR)lpChar);			// BUG
+
+	lpChar = (LPSTR)"a"; // Valid
+	lpWchar = (LPWSTR)L"a"; // Valid
+
+	fconstWChar((LPCWSTR)lpWchar);	// Valid
+	fWChar(lpWchar);				// Valid
+}

cpp/ql/test/query-tests/Security/CWE/CWE-704/WcharCharConversion.expected

@@ -0,0 +1,5 @@
+| WcharCharConversion.cpp:21:20:21:22 | array to pointer conversion | Conversion from const char * to LPWSTR. Use of invalid string can lead to undefined behavior. |
+| WcharCharConversion.cpp:22:20:22:25 | lpcstr | Conversion from LPCSTR to LPWSTR. Use of invalid string can lead to undefined behavior. |
+| WcharCharConversion.cpp:24:22:24:27 | lpChar | Conversion from char * to wchar_t *. Use of invalid string can lead to undefined behavior. |
+| WcharCharConversion.cpp:26:23:26:28 | lpChar | Conversion from char * to LPCWSTR. Use of invalid string can lead to undefined behavior. |
+| WcharCharConversion.cpp:27:17:27:22 | lpChar | Conversion from char * to LPWSTR. Use of invalid string can lead to undefined behavior. |

cpp/ql/test/query-tests/Security/CWE/CWE-704/WcharCharConversion.qlref

@@ -0,0 +1 @@
+Security/CWE/CWE-704/WcharCharConversion.ql