[cfe-commits] [PATCH] review request - strcat modeling addition to CStringChecker

[Ted Kremenek]

Hi Lenny,

I'm a bit dubious about the following:

+  // ultimately contain both.
+  if (isAppending) {
+    // Get the string length of the destination, or give up.
+    SVal dstStrLength = getCStringLength(C, state, Dst, DstVal);
+    if (dstStrLength.isUndef())
+      return;
+
+    NonLoc *srcStrLengthNL = dyn_cast<NonLoc>(&strLength);
+    NonLoc *dstStrLengthNL = dyn_cast<NonLoc>(&dstStrLength);
+
+    QualType addTy = C.getSValBuilder().getContext().IntTy;
+
+    strLength = C.getSValBuilder().evalBinOpNN(state, BO_Add, 
+                                               *srcStrLengthNL, *dstStrLengthNL,
+                                               addTy);
+  }

The dyn_cast<> followed by the unguarded call to 'evalBinOpNN' looks wrong.  There is no guarantee that those values are non-null (which is why I assume you used a dyn_cast<>).  This looks like a potential null dereference.


On Apr 3, 2011, at 10:35 AM, Lenny Maiorani wrote:

> strcat() model addition to CStringChecker.
> 
> Validates inputs are not NULL, checks for overlapping strings, concatenates the strings checking for buffer overflow, sets the length of the destination string to the sum of the s1 length and the s2 length, binds the return value to the s1 value.
> 
> 
> -Lenny
> 
> <strcat-modeling-checker.diff>
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits