Enhance cpp/overflow-calculated - detect out-of-bounds write caused by passing the buffer size in bytes (using sizeof) instead of the number of elements to wcsftime, allowing the function to overrun the allocated buffer.

cpp/ql/src/Critical/OverflowCalculated.ql

@@ -1,7 +1,8 @@
 /**
- * @name Buffer not sufficient for string
- * @description A buffer allocated using 'malloc' may not have enough space for a string that is being copied into it. The operation can cause a buffer overrun. Make sure that the buffer contains enough room for the string (including the zero terminator).
+ * @name Buffer overflow from insufficient space or incorrect size calculation
+ * @description A buffer allocated using 'malloc' may not have enough space for a string being copied into it, or wide character functions may receive incorrect size parameters causing buffer overrun. Make sure that buffers contain enough room for strings (including zero terminator) and that size parameters are correctly calculated.
  * @kind problem
+ * @precision medium
  * @id cpp/overflow-calculated
  * @problem.severity warning
  * @security-severity 9.8
@@ -40,6 +41,38 @@ predicate spaceProblem(FunctionCall append, string msg) {
   )
 }
 
+predicate wideCharSizeofProblem(FunctionCall call, string msg) {
+  exists(
+    Variable buffer, SizeofExprOperator sizeofOp
+  |
+    // Function call is to wcsftime
+    call.getTarget().hasGlobalOrStdName("wcsftime") and
+    // Second argument (count parameter) is a sizeof operation
+    call.getArgument(1) = sizeofOp and
+    // The sizeof is applied to a buffer variable
+    sizeofOp.getExprOperand() = buffer.getAnAccess() and
+    (
+      // Case 1: Array of wchar_t - sizeof gives bytes instead of element count
+      exists(ArrayType arrayType |
+        arrayType = buffer.getType() and
+        arrayType.getBaseType().hasName("wchar_t") and
+        msg =
+          "Using sizeof(" + buffer.getName() + ") passes byte count instead of wchar_t element count to wcsftime. " +
+          "Use sizeof(" + buffer.getName() + ")/sizeof(wchar_t) or array length instead."
+      )
+      or
+      // Case 2: Pointer to wchar_t - sizeof gives pointer size, which is completely wrong
+      exists(PointerType ptrType |
+        ptrType = buffer.getType() and
+        ptrType.getBaseType().hasName("wchar_t") and
+        msg =
+          "Using sizeof(" + buffer.getName() + ") passes pointer size instead of buffer size to wcsftime. " +
+          "Pass the actual element count or use a length variable instead."
+      )
+    )
+  )
+}
+
 from Expr problem, string msg
-where spaceProblem(problem, msg)
+where spaceProblem(problem, msg) or wideCharSizeofProblem(problem, msg)
 select problem, msg

cpp/ql/src/change-notes/2025-06-11-add-overflow-calculated-to-security-extended.md

@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* Adds `cpp/overflow-calculated` to the `cpp-security-extended` query suite.

cpp/ql/test/query-tests/Critical/OverflowCalculated/OverflowCalculated.expected

@@ -1,2 +1,5 @@
-| tests2.cpp:34:4:34:9 | call to strcat | This buffer only contains enough room for 'str1' (copied on line 33) |
-| tests2.cpp:52:4:52:9 | call to strcat | This buffer only contains enough room for 'str1' (copied on line 51) |
+| tests2.cpp:48:4:48:9 | call to strcat | This buffer only contains enough room for 'str1' (copied on line 47) |
+| tests2.cpp:66:4:66:9 | call to strcat | This buffer only contains enough room for 'str1' (copied on line 65) |
+| tests2.cpp:118:4:118:11 | call to wcsftime | Using sizeof(buf) passes byte count instead of wchar_t element count to wcsftime. Use sizeof(buf)/sizeof(wchar_t) or array length instead. |
+| tests2.cpp:142:4:142:11 | call to wcsftime | Using sizeof(smallBuf) passes byte count instead of wchar_t element count to wcsftime. Use sizeof(smallBuf)/sizeof(wchar_t) or array length instead. |
+| tests2.cpp:151:5:151:12 | call to wcsftime | Using sizeof(dynamicBuf) passes pointer size instead of buffer size to wcsftime. Pass the actual element count or use a length variable instead. |

cpp/ql/test/query-tests/Critical/OverflowCalculated/tests2.cpp

@@ -2,8 +2,22 @@
 
 typedef unsigned int size_t;
 
+// Time structures and functions for wcsftime tests
+struct tm {
+    int tm_sec;
+    int tm_min;
+    int tm_hour;
+    int tm_mday;
+    int tm_mon;
+    int tm_year;
+    int tm_wday;
+    int tm_yday;
+    int tm_isdst;
+};
+
 size_t strlen(const char *str);
 size_t wcslen(const wchar_t *wcs);
+size_t wcsftime(wchar_t *strDest, size_t maxsize, const wchar_t *format, const struct tm *timeptr);
 
 char *strcpy(char *destination, const char *source);
 wchar_t *wcscpy(wchar_t *strDestination, const wchar_t *strSource);
@@ -95,6 +109,61 @@ void tests2(int case_num)
 			wcscpy(wbuffer, wstr1);
 			wcscat(wbuffer, wstr2);
 			break;
+
+		// wcsftime test cases
+		case 200:
+		{
+			wchar_t buf[80];
+			struct tm timeinfo = {0};
+			wcsftime(buf, sizeof(buf), L"%Y-%m-%d %H:%M:%S", &timeinfo); // BAD: sizeof(buf) returns bytes, not wchar_t count
+			break;
+		}
+
+		case 201:
+		{
+			wchar_t buf[80];
+			struct tm timeinfo = {0};
+			wcsftime(buf, sizeof(buf) / sizeof(wchar_t), L"%Y-%m-%d %H:%M:%S", &timeinfo); // GOOD: correct element count
+			break;
+		}
+
+		case 202:
+		{
+			wchar_t buf[80];
+			struct tm timeinfo = {0};
+			wcsftime(buf, 80, L"%Y-%m-%d %H:%M:%S", &timeinfo); // GOOD: direct array length
+			break;
+		}
+
+		case 203:
+		{
+			wchar_t smallBuf[20];
+			struct tm timeinfo = {0};
+			wcsftime(smallBuf, sizeof(smallBuf), L"%Y-%m-%d %H:%M:%S", &timeinfo); // BAD: sizeof returns bytes
+			break;
+		}
+
+		case 204:
+		{
+			wchar_t *dynamicBuf = (wchar_t *)malloc(50 * sizeof(wchar_t));
+			struct tm timeinfo = {0};
+			if (dynamicBuf) {
+				wcsftime(dynamicBuf, sizeof(dynamicBuf), L"%Y-%m-%d %H:%M:%S", &timeinfo); // BAD: sizeof on pointer
+				free(dynamicBuf);
+			}
+			break;
+		}
+
+		case 205:
+		{
+			wchar_t *dynamicBuf = (wchar_t *)malloc(50 * sizeof(wchar_t));
+			struct tm timeinfo = {0};
+			if (dynamicBuf) {
+				wcsftime(dynamicBuf, 50, L"%Y-%m-%d %H:%M:%S", &timeinfo); // GOOD: correct element count
+				free(dynamicBuf);
+			}
+			break;
+		}
 	}
 
 	if (buffer != 0)