Merge pull request #293 from geoffw0/zerosizebuffer

CPP: Better handling of zero-sized buffers

change-notes/1.19/analysis-cpp.md

@@ -28,3 +28,4 @@
 
 * Added a hash consing library for structural comparison of expressions.
 * `getBufferSize` now detects variable size structs more reliably.
+* Buffer.qll now treats arrays of zero size as a special case.

cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgs.ql

@@ -104,9 +104,13 @@ where
   // Some of the functions operate on a larger char type, like `wchar_t`, so we
   // need to take this into account in the fixed size case.
   charSize = f.getParameter(argDest).getType().getUnspecifiedType().(PointerType).getBaseType().getSize() and
-  if exists (fc.getArgument(argLimit).getValue().toInt()) then (
+  if exists(fc.getArgument(argLimit).getValue().toInt()) then (
     // Fixed sized case
-    arrayExprFixedSize(copyDest) < charSize * fc.getArgument(argLimit).getValue().toInt()
+    exists(int size |
+      size = arrayExprFixedSize(copyDest) and
+      size < charSize * fc.getArgument(argLimit).getValue().toInt() and
+      size != 0 // if the array has zero size, something special is going on
+    )
   ) else exists (Access takenSizeOf, BufferSizeExpr sizeExpr, int plus |
     // Variable sized case
     sizeExpr = fc.getArgument(argLimit).getAChild*() and

cpp/ql/src/semmle/code/cpp/commons/Buffer.qll

@@ -57,7 +57,9 @@ int getBufferSize(Expr bufferExpr, Element why) {
       // buffer is a fixed size array
       result = bufferVar.getType().getUnspecifiedType().(ArrayType).getSize() and
       why = bufferVar and
-      not memberMayBeVarSize(_, bufferVar)
+      not memberMayBeVarSize(_, bufferVar) and
+      not result = 0 // zero sized arrays are likely to have special usage, for example
+                     // behaving a bit like a 'union' overlapping other fields. 
     ) or (
       // buffer is an initialized array
       //  e.g. int buffer[] = {1, 2, 3};