Merge pull request #226 from elbiazo/bad_wchar

adding bad wchar query

Author: dilanbhalla

cpp/ql/src/Microsoft/Likely Bugs/Likely Typos/BadWchar.cpp

@@ -0,0 +1,16 @@
+// Query should catch invalidWchar1 and invalidWchar2 but not semiValidWchar and validWchar
+const WCHAR invalidWchar3[] = {
+    L'C', L'P', L'R', L'O', L'G', L'R', L'A', L'M', L' ', L'1', L'.', L'0',
+    'L\0'};
+
+const WCHAR invalidWchar2[] = {
+    L'C', L'P', L'R', L'O', L'G', L'R', L'A', L'M', L' ', L'1', L'.', L'0',
+    'LZ'};
+
+const WCHAR semiValidWchar[] = {
+    L'C', L'P', L'R', L'O', L'G', L'R', L'A', L'M', L' ', L'1', L'.', L'0',
+    L'L\0'};
+
+const WCHAR validWchar[] = {
+    L'C', L'P', L'R', L'O', L'G', L'R', L'A', L'M', L' ', L'1', L'.', L'0',
+    L'\0'};

cpp/ql/src/Microsoft/Likely Bugs/Likely Typos/BadWchar.qhelp

@@ -0,0 +1,15 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+    <overview>
+        <p>This rule finds wide character that is initialized wrong due to a typo.
+        </p>
+    </overview>
+    <recommendation>
+        <p>The wide character should be initialized with the L prefix. If L is accidentally included
+            inside the character literal, it should be moved outside to be a prefix.
+        </p>
+    </recommendation>
+    <example>
+        <sample src="BadWchar.cpp" />
+    </example>
+</qhelp>

cpp/ql/src/Microsoft/Likely Bugs/Likely Typos/BadWchar.ql

@@ -0,0 +1,40 @@
+/**
+ * @id cpp/microsoft/public/typo/bad-wchar
+ * @name Initialization of bad wide char
+ * @description wchar_t should be initialized with L prefix but might be initialized inside single quote. E.g. 'L\0' instead of L'\0'
+ * @kind problem
+ * @problem.severity error
+ * @precision high
+ * @tags security
+ */
+
+import cpp
+
+/**
+ * Check where wchar_t is initalized wrong.
+ * E.g. wchar_t a = 'LZ' (bad) instead of L'Z' (good)
+ */
+predicate badWchar(CharLiteral cl) {
+  cl.getActualType().hasName("wchar_t") and
+  cl.getValueText().regexpMatch("'L.+'") // wchar_t getValueText() generally looks like L'Z' or 'L\0'. but if it is 'LZ' or 'L\0', this might be bad
+}
+
+/**
+ * when codeql can't figure out what underlying type it is, it will default to int. Scenarios unique_ptr, it seems to have hard time figuring out...
+ * we can check for others like 'LZ' or 'LA' but you will get alot of false positive since there can be enum :int with 'LZ' field etc
+ * E.g. unique_ptr<PWSTR>('L\0'). If codeql couldnt figureout what unique_ptr is pointing to it will set it int.
+ */
+predicate wcharNullOnNonWchar(CharLiteral cl) {
+  cl.getValue().toInt() = 19456 // 19456 (0x4c00) is 'L\0'
+}
+
+from CharLiteral cl, string msg
+where
+  (
+    wcharNullOnNonWchar(cl) or
+    badWchar(cl)
+  ) and
+  msg =
+    " Varible of type wchar_t maybe have been initialized with a typo. " + cl.getValueText() +
+      " should be L'" + cl.getValueText().substring(2, cl.getValueText().length())
+select cl, "$@: " + msg, cl, cl.getValueText()

cpp/ql/test/query-tests/Microsoft/Likely Bugs/Likely Typos/BadWchar/BadWchar.expected

@@ -0,0 +1,2 @@
+| test.cpp:8:9:8:13 | 19456 | $@:  Varible of type wchar_t maybe have been initialized with a typo. 'L\\0' should be L'\\0' | test.cpp:8:9:8:13 | 19456 | 'L\\0' |
+| test.cpp:12:9:12:12 | 19546 | $@:  Varible of type wchar_t maybe have been initialized with a typo. 'LZ' should be L'Z' | test.cpp:12:9:12:12 | 19546 | 'LZ' |

cpp/ql/test/query-tests/Microsoft/Likely Bugs/Likely Typos/BadWchar/BadWchar.qlref

@@ -0,0 +1 @@
+Microsoft/Likely Bugs/Likely Typos/BadWchar.ql

cpp/ql/test/query-tests/Microsoft/Likely Bugs/Likely Typos/BadWchar/test.cpp

@@ -0,0 +1,24 @@
+// Query should catch invalidWchar1 and invalidWchar2 in invalidWcharTest function but not semiValidWchar and validWchar in validWcharTest function
+typedef wchar_t WCHAR;
+
+void invalidWcharTest()
+{
+    const WCHAR invalidWchar3[] = {
+        L'C', L'P', L'R', L'O', L'G', L'R', L'A', L'M', L' ', L'1', L'.', L'0',
+        'L\0'}; // 'L\0' should be L'\0'
+
+    const WCHAR invalidWchar2[] = {
+        L'C', L'P', L'R', L'O', L'G', L'R', L'A', L'M', L' ', L'1', L'.', L'0',
+        'LZ'}; // 'LZ' should be L'Z'
+}
+
+void validWcharTest()
+{
+    const WCHAR semiValidWchar[] = {
+        L'C', L'P', L'R', L'O', L'G', L'R', L'A', L'M', L' ', L'1', L'.', L'0',
+        L'L\0'};
+
+    const WCHAR validWchar[] = {
+        L'C', L'P', L'R', L'O', L'G', L'R', L'A', L'M', L' ', L'1', L'.', L'0',
+        L'\0'};
+}