[cfe-commits] r80313 - in /cfe/trunk: lib/Analysis/CheckSecuritySyntaxOnly.cpp test/Analysis/security-syntax-checks.m
Ted Kremenek
kremenek at apple.com
Thu Aug 27 17:08:09 PDT 2009
Author: kremenek
Date: Thu Aug 27 19:08:09 2009
New Revision: 80313
URL: http://llvm.org/viewvc/llvm-project?rev=80313&view=rev
Log:
Implement: <rdar://problem/6337132> CWE-273: Failure to Check Whether Privileges
Were Dropped Successfully
Patch by Geoff Keating!
Modified:
cfe/trunk/lib/Analysis/CheckSecuritySyntaxOnly.cpp
cfe/trunk/test/Analysis/security-syntax-checks.m
Modified: cfe/trunk/lib/Analysis/CheckSecuritySyntaxOnly.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CheckSecuritySyntaxOnly.cpp?rev=80313&r1=80312&r2=80313&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/CheckSecuritySyntaxOnly.cpp (original)
+++ cfe/trunk/lib/Analysis/CheckSecuritySyntaxOnly.cpp Thu Aug 27 19:08:09 2009
@@ -22,14 +22,18 @@
namespace {
class VISIBILITY_HIDDEN WalkAST : public StmtVisitor<WalkAST> {
BugReporter &BR;
- IdentifierInfo *II_gets;
+ IdentifierInfo *II_gets;
+ enum { num_ids = 6 };
+ IdentifierInfo *II_setid[num_ids];
+
public:
WalkAST(BugReporter &br) : BR(br),
- II_gets(0) {}
+ II_gets(0), II_setid() {}
// Statement visitor methods.
void VisitCallExpr(CallExpr *CE);
void VisitForStmt(ForStmt *S);
+ void VisitCompoundStmt (CompoundStmt *S);
void VisitStmt(Stmt *S) { VisitChildren(S); }
void VisitChildren(Stmt *S);
@@ -40,6 +44,7 @@
// Checker-specific methods.
void CheckLoopConditionForFloat(const ForStmt *FS);
void CheckCall_gets(const CallExpr *CE, const FunctionDecl *FD);
+ void CheckUncheckedReturnValue(CallExpr *CE);
};
} // end anonymous namespace
@@ -73,6 +78,16 @@
VisitChildren(CE);
}
+void WalkAST::VisitCompoundStmt(CompoundStmt *S) {
+ for (Stmt::child_iterator I = S->child_begin(), E = S->child_end(); I!=E; ++I)
+ if (Stmt *child = *I)
+ {
+ if (CallExpr *CE = dyn_cast<CallExpr>(child))
+ CheckUncheckedReturnValue(CE);
+ Visit(child);
+ }
+}
+
void WalkAST::VisitForStmt(ForStmt *FS) {
CheckLoopConditionForFloat(FS);
@@ -226,6 +241,69 @@
}
//===----------------------------------------------------------------------===//
+// Check: Should check whether privileges are dropped successfully.
+// Originally: <rdar://problem/6337132>
+//===----------------------------------------------------------------------===//
+
+void WalkAST::CheckUncheckedReturnValue(CallExpr *CE) {
+ const FunctionDecl *FD = CE->getDirectCallee();
+ if (!FD)
+ return;
+
+ if (II_setid[0] == NULL) {
+ static const char * const identifiers[num_ids] = {
+ "setuid", "setgid", "seteuid", "setegid",
+ "setreuid", "setregid"
+ };
+
+ for (size_t i = 0; i < num_ids; i++)
+ II_setid[i] = &BR.getContext().Idents.get(identifiers[i]);
+ }
+
+ const IdentifierInfo *id = FD->getIdentifier();
+ size_t identifierid;
+
+ for (identifierid = 0; identifierid < num_ids; identifierid++)
+ if (id == II_setid[identifierid])
+ break;
+
+ if (identifierid >= num_ids)
+ return;
+
+ const FunctionProtoType *FTP = dyn_cast<FunctionProtoType>(FD->getType());
+ if (!FTP)
+ return;
+
+ /* Verify that the function takes one or two arguments (depending on
+ the function). */
+ if (FTP->getNumArgs() != (identifierid < 4 ? 1 : 2))
+ return;
+
+ // The arguments must be integers.
+ for (unsigned i = 0; i < FTP->getNumArgs(); i++)
+ if (! FTP->getArgType(i)->isIntegerType())
+ return;
+
+ // Issue a warning.
+ std::string buf1;
+ llvm::raw_string_ostream os1(buf1);
+ os1 << "Return value is not checked in call to '" << FD->getNameAsString()
+ << "'";
+
+ std::string buf2;
+ llvm::raw_string_ostream os2(buf2);
+ os2 << "The return value from the call to '" << FD->getNameAsString()
+ << "' is not checked. If an error occurs in '"
+ << FD->getNameAsString()
+ << "', the following code may execute with unexpected privileges";
+
+ SourceRange R = CE->getCallee()->getSourceRange();
+
+ BR.EmitBasicReport(os1.str().c_str(), "Security", os2.str().c_str(),
+ CE->getLocStart(), &R, 1);
+}
+
+//===----------------------------------------------------------------------===//
// Entry point for check.
//===----------------------------------------------------------------------===//
Modified: cfe/trunk/test/Analysis/security-syntax-checks.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/security-syntax-checks.m?rev=80313&r1=80312&r2=80313&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/security-syntax-checks.m (original)
+++ cfe/trunk/test/Analysis/security-syntax-checks.m Thu Aug 27 19:08:09 2009
@@ -1,4 +1,4 @@
-// RUN: clang-cc -analyze -warn-security-syntactic %s -verify
+// RUN: clang-cc -triple i386-apple-darwin10 -analyze -warn-security-syntactic %s -verify
// <rdar://problem/6336718> rule request: floating point used as loop
// condition (FLP30-C, FLP-30-CPP)
@@ -29,3 +29,34 @@
char buff[1024];
gets(buff); // expected-warning{{Call to function 'gets' is extremely insecure as it can always result in a buffer overflow}}
}
+
+// <rdar://problem/6337132> CWE-273: Failure to Check Whether Privileges Were
+// Dropped Successfully
+typedef unsigned int __uint32_t;
+typedef __uint32_t __darwin_uid_t;
+typedef __uint32_t __darwin_gid_t;
+typedef __darwin_uid_t uid_t;
+typedef __darwin_gid_t gid_t;
+int setuid(uid_t);
+int setregid(gid_t, gid_t);
+int setreuid(uid_t, uid_t);
+extern void check(int);
+
+void test_setuid()
+{
+ setuid(2); // expected-warning{{The return value from the call to 'setuid' is not checked. If an error occurs in 'setuid', the following code may execute with unexpected privileges}}
+ setuid(0); // expected-warning{{The return value from the call to 'setuid' is not checked. If an error occurs in 'setuid', the following code may execute with unexpected privileges}}
+ if (setuid (2) != 0)
+ abort();
+
+ // Currently the 'setuid' check is not flow-sensitive, and only looks
+ // at whether the function was called in a compound statement. This
+ // will lead to false negatives, but there should be no false positives.
+ int t = setuid(2); // no-warning
+ (void)setuid (2); // no-warning
+
+ check(setuid (2)); // no-warning
+
+ setreuid(2,2); // expected-warning{{The return value from the call to 'setreuid' is not checked. If an error occurs in 'setreuid', the following code may execute with unexpected privileges}}
+ setregid(2,2); // expected-warning{{The return value from the call to 'setregid' is not checked. If an error occurs in 'setregid', the following code may execute with unexpected privileges}}
+}
More information about the cfe-commits
mailing list