Add Tests and handle additional scenario:

- Pointer case: sizeof(pointer) gives pointer size (e.g., 8 bytes), which is completely wrong

Author: felickz

cpp/ql/src/Critical/OverflowCalculated.ql

@@ -2,7 +2,6 @@
  * @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
@@ -43,20 +42,33 @@ predicate spaceProblem(FunctionCall append, string msg) {
 
 predicate wideCharSizeofProblem(FunctionCall call, string msg) {
   exists(
-    Variable buffer, SizeofExprOperator sizeofOp, ArrayType arrayType
+    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
-    // The buffer is an array of wchar_t
-    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."
+    (
+      // 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."
+      )
+    )
   )
 }
 

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)