Merge pull request #15448 from microsoft/false_positive_cpp_incorrect_string_type_conversion

cpp/incorrect-string-type-conversion false positive fixes

Author: MathiasVP

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

@@ -18,13 +18,31 @@ class WideCharPointerType extends PointerType {
   WideCharPointerType() { this.getBaseType() instanceof WideCharType }
 }
 
+/**
+ * A type that may also be `CharPointerType`, but that are likely used as arbitrary buffers.
+ */
+class UnlikelyToBeAStringType extends Type {
+  UnlikelyToBeAStringType() {
+    this.(PointerType).getBaseType().(CharType).isUnsigned() or
+    this.(PointerType).getBaseType().getName().toLowerCase().matches("%byte") or
+    this.getName().toLowerCase().matches("%byte") or
+    this.(PointerType).getBaseType().hasName("uint8_t")
+  }
+}
+
 from Expr e1, Cast e2
 where
   e2 = e1.getConversion() and
   exists(WideCharPointerType w, CharPointerType c |
     w = e2.getUnspecifiedType().(PointerType) and
     c = e1.getUnspecifiedType().(PointerType)
-  )
+  ) and
+  // Avoid `BYTE`-like casting as they are typically false positives
+  // Example: `BYTE* buffer;` ... `(wchar_t*) buffer;`
+  not e1.getType() instanceof UnlikelyToBeAStringType and
+  // Avoid castings from 'new' expressions as typically these will be safe
+  // Example: `__Type* ret = reinterpret_cast<__Type*>(New(m_pmo) char[num * sizeof(__Type)]);`
+  not exists(NewOrNewArrayExpr newExpr | newExpr.getAChild*() = e1)
 select e1,
   "Conversion from " + e1.getType().toString() + " to " + e2.getType().toString() +
     ". Use of invalid string can lead to undefined behavior."

cpp/ql/src/change-notes/2024-01-29-false_positive_incorrect_string_type_conversion.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Corrected 2 false positive with `cpp/incorrect-string-type-conversion`: conversion of byte arrays to wchar and new array allocations converted to wchar.

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

@@ -31,4 +31,26 @@ void Test()
 
 	fconstWChar((LPCWSTR)lpWchar);	// Valid
 	fWChar(lpWchar);				// Valid
+}
+
+void NewBufferFalsePositiveTest()
+{
+	wchar_t *lpWchar = NULL;
+
+	lpWchar = (LPWSTR)new char[56]; // Possible False Positive
+}
+
+typedef unsigned char BYTE;
+typedef BYTE* PBYTE;
+
+void NonStringFalsePositiveTest1(PBYTE buffer)
+{
+	wchar_t *lpWchar = NULL;
+	lpWchar = (LPWSTR)buffer; // Possible False Positive
+}
+
+void NonStringFalsePositiveTest2(unsigned char* buffer)
+{
+	wchar_t *lpWchar = NULL;
+	lpWchar = (LPWSTR)buffer; // Possible False Positive
 }