Hi Arthur,
Sorry for not updating the patch last time. I was waiting for your comments
before updating it. Updated the patch now.
And for this case:
char x[3] = "abc";
char y[100];
strcpy(y, x);
I had mentioned that this does not seem to be buffer-overflow, as when i
checked the same with clang, strcpy is inserting a null terminator after
copying the contents of source array.
$ cat strcpy.c
#include<string.h>
#include<stdio.h>
int main ()
{
char x[3]; // non-null terminated array
x[0] = 'a';
x[1] = 'b';
x[2] = 'c';
char y[100] ;
memset(y,'a',100);
strcpy(y,x);
printf("%s \n",y);
printf("%c\n",y[0]);
printf("%c\n",y[1]);
printf("%c\n",y[2]);
printf("%c\n",y[3]);
printf("%c\n",y[4]);
return 0;
}
$ clang strcpy.c
$ ./a.out
abc
a
b
c
a
In this example it is seen that clang is inserting null terminator and hence
even while copying non-null terminated string to another array, buffer-overflow
is not caused.
Please provide comments on whether this analysis is correct or not.
Thanks,
Mayur
http://reviews.llvm.org/D6012
Files:
lib/StaticAnalyzer/Checkers/CStringChecker.cpp
test/Analysis/string.c
Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -39,6 +39,7 @@
BT_NotCString, BT_AdditionOverflow;
mutable const char *CurrentFunctionDescription;
+ static bool isSrcGarbage;
public:
/// The filter is used to filter out the diagnostics which are not enabled by
@@ -195,6 +196,8 @@
NonLoc right) const;
};
+bool CStringChecker::isSrcGarbage = 0;
+
} //end anonymous namespace
REGISTER_MAP_WITH_PROGRAMSTATE(CStringLength, const MemRegion *, SVal)
@@ -667,6 +670,10 @@
const SVal *Recorded = state->get<CStringLength>(MR);
if (Recorded)
return *Recorded;
+ else {
+ // set isSrcGarbage, so that we will try to evaluate size of src for strcpy fn
+ isSrcGarbage = 1;
+ }
}
// Otherwise, get a new symbol and update the state.
@@ -1360,6 +1367,35 @@
QualType cmpTy = svalBuilder.getConditionType();
QualType sizeTy = svalBuilder.getContext().getSizeType();
+ // If we could not record proper string length, try to get the size of src buffer
+ if (isSrcGarbage && !isAppending && !isBounded && !returnEnd) {
+ isSrcGarbage = 0;
+ const MemRegion *R = srcVal.getAsRegion();
+ if (!R)
+ return;
+
+ const ElementRegion *ER = dyn_cast<ElementRegion>(R);
+ SVal retsize = UnknownVal();
+ if (ER) {
+ assert(ER->getValueType() == C.getASTContext().CharTy &&
+ "Should only be called with char* ElementRegions");
+
+ // Get the size of the array.
+ const SubRegion *superReg = cast<SubRegion>(ER->getSuperRegion());
+ SVal Extent = svalBuilder.convertToArrayIndex(superReg->getExtent(svalBuilder));
+ DefinedOrUnknownSVal strSize = cast<DefinedOrUnknownSVal>(Extent);
+ retsize = strSize;
+
+ NonLoc One = svalBuilder.makeIntVal(1, sizeTy).castAs<NonLoc>();
+ NonLoc *knownStrSize = cast<NonLoc>(&retsize);
+ retsize = (svalBuilder.evalBinOpNN(state, BO_Sub, *knownStrSize, One, sizeTy));
+
+ if (!(retsize.isUnknown()) && !(retsize.isUndef())) {
+ strLength = retsize;
+ }
+ }
+ }
+
// These two values allow checking two kinds of errors:
// - actual overflows caused by a source that doesn't fit in the destination
// - potential overflows caused by a bound that could exceed the destination
Index: test/Analysis/string.c
===================================================================
--- test/Analysis/string.c
+++ test/Analysis/string.c
@@ -303,6 +303,14 @@
strcpy(x, y); // no-warning
}
+extern void * memset(void *, int , unsigned long );
+void strcpy_src_garbage_overflow1() {
+ char x[4] = "abc";
+ char y[5];
+ memset(y,'a',5);
+ strcpy(x,y); // expected-warning{{String copy function overflows destination buffer}}
+}
+
//===----------------------------------------------------------------------===
// stpcpy()
//===----------------------------------------------------------------------===
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
