Hi, These patches makes the unix.API checker detect more errors when using open(2). The first is to detect when open is called with more than three arguments. The second patch makes sure that the third argument is an integer.
I've added a new test file for these cases. The current one, nix-fns.c, does a lot more than simply testing the expected warnings. Cheers, Daniel Fahlgren
>From 47def5de23149b72bdf50491e84aee12fcd99312 Mon Sep 17 00:00:00 2001 From: Daniel Fahlgren <[email protected]> Date: Mon, 18 Aug 2014 12:37:38 +0200 Subject: [PATCH 1/2] Warn when open is called with too many arguments --- lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp | 57 +++++++++++++++--------- test/Analysis/unix-api.c | 27 +++++++++++ 2 files changed, 63 insertions(+), 21 deletions(-) create mode 100644 test/Analysis/unix-api.c diff --git a/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp index 4887d80..e7626ca 100644 --- a/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp @@ -62,6 +62,10 @@ private: return; BT.reset(new BugType(this, name, categories::UnixAPI)); } + void ReportOpenBug(CheckerContext &C, + ProgramStateRef State, + const char *Msg, + SourceRange SR) const; }; } //end anonymous namespace @@ -69,7 +73,35 @@ private: // "open" (man 2 open) //===----------------------------------------------------------------------===// +void UnixAPIChecker::ReportOpenBug(CheckerContext &C, + ProgramStateRef State, + const char *Msg, + SourceRange SR) const { + ExplodedNode *N = C.generateSink(State); + if (!N) + return; + + LazyInitialize(BT_open, "Improper use of 'open'"); + + BugReport *Report = new BugReport(*BT_open, Msg, N); + Report->addRange(SR); + C.emitReport(Report); +} + void UnixAPIChecker::CheckOpen(CheckerContext &C, const CallExpr *CE) const { + ProgramStateRef state = C.getState(); + + if (CE->getNumArgs() < 2) { + // The frontend should issue a warning for this case, so this is a sanity + // check. + return; + } else if (CE->getNumArgs() > 3) { + ReportOpenBug(C, state, + "Call to 'open' with more than three arguments", + CE->getArg(3)->getSourceRange()); + return; + } + // The definition of O_CREAT is platform specific. We need a better way // of querying this information from the checking environment. if (!Val_O_CREAT.hasValue()) { @@ -85,15 +117,6 @@ void UnixAPIChecker::CheckOpen(CheckerContext &C, const CallExpr *CE) const { } } - // Look at the 'oflags' argument for the O_CREAT flag. - ProgramStateRef state = C.getState(); - - if (CE->getNumArgs() < 2) { - // The frontend should issue a warning for this case, so this is a sanity - // check. - return; - } - // Now check if oflags has O_CREAT set. const Expr *oflagsEx = CE->getArg(1); const SVal V = state->getSVal(oflagsEx, C.getLocationContext()); @@ -122,18 +145,10 @@ void UnixAPIChecker::CheckOpen(CheckerContext &C, const CallExpr *CE) const { return; if (CE->getNumArgs() < 3) { - ExplodedNode *N = C.generateSink(trueState); - if (!N) - return; - - LazyInitialize(BT_open, "Improper use of 'open'"); - - BugReport *report = - new BugReport(*BT_open, - "Call to 'open' requires a third argument when " - "the 'O_CREAT' flag is set", N); - report->addRange(oflagsEx->getSourceRange()); - C.emitReport(report); + ReportOpenBug(C, trueState, + "Call to 'open' requires a third argument when " + "the 'O_CREAT' flag is set", + oflagsEx->getSourceRange()); } } diff --git a/test/Analysis/unix-api.c b/test/Analysis/unix-api.c new file mode 100644 index 0000000..4146822 --- /dev/null +++ b/test/Analysis/unix-api.c @@ -0,0 +1,27 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.API -verify %s + +#ifndef O_RDONLY +#define O_RDONLY 0 +#endif + +#ifndef NULL +#define NULL ((void*) 0) +#endif + +int open(const char *, int, ...); +int close(int fildes); + +void open_1(const char *path) { + int fd; + fd = open(path, O_RDONLY); // no-warning + if (fd > -1) + close(fd); +} + +void open_2(const char *path) { + int fd; + int mode = 0x0; + fd = open(path, O_RDONLY, mode, NULL); // expected-warning{{Call to 'open' with more than three arguments}} + if (fd > -1) + close(fd); +} -- 1.7.9.5
>From c817b1a23b9f21da3ede9cfe76631362ab07d3b8 Mon Sep 17 00:00:00 2001 From: Daniel Fahlgren <[email protected]> Date: Mon, 18 Aug 2014 12:38:40 +0200 Subject: [PATCH 2/2] Check if open(2) is called with a valid mode When creating files open(2) requires a third argument. Make sure that argument is an integer. --- lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp | 9 +++++ test/Analysis/unix-api.c | 48 ++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp index e7626ca..4bfed85 100644 --- a/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp @@ -95,6 +95,15 @@ void UnixAPIChecker::CheckOpen(CheckerContext &C, const CallExpr *CE) const { // The frontend should issue a warning for this case, so this is a sanity // check. return; + } else if (CE->getNumArgs() == 3) { + const Expr *Arg = CE->getArg(2); + QualType QT = Arg->getType(); + if (!QT->isIntegerType()) { + ReportOpenBug(C, state, + "Third argument to 'open' is not an integer", + Arg->getSourceRange()); + return; + } } else if (CE->getNumArgs() > 3) { ReportOpenBug(C, state, "Call to 'open' with more than three arguments", diff --git a/test/Analysis/unix-api.c b/test/Analysis/unix-api.c index 4146822..86c702d 100644 --- a/test/Analysis/unix-api.c +++ b/test/Analysis/unix-api.c @@ -25,3 +25,51 @@ void open_2(const char *path) { if (fd > -1) close(fd); } + +void open_3(const char *path) { + int fd; + fd = open(path, O_RDONLY, NULL); // expected-warning{{Third argument to 'open' is not an integer}} + if (fd > -1) + close(fd); +} + +void open_4(const char *path) { + int fd; + fd = open(path, O_RDONLY, ""); // expected-warning{{Third argument to 'open' is not an integer}} + if (fd > -1) + close(fd); +} + +void open_5(const char *path) { + int fd; + struct { + int val; + } st = {0}; + fd = open(path, O_RDONLY, st); // expected-warning{{Third argument to 'open' is not an integer}} + if (fd > -1) + close(fd); +} + +void open_6(const char *path) { + int fd; + struct { + int val; + } st = {0}; + fd = open(path, O_RDONLY, st.val); // no-warning + if (fd > -1) + close(fd); +} + +void open_7(const char *path) { + int fd; + fd = open(path, O_RDONLY, &open); // expected-warning{{Third argument to 'open' is not an integer}} + if (fd > -1) + close(fd); +} + +void open_8(const char *path) { + int fd; + fd = open(path, O_RDONLY, 0.0f); // expected-warning{{Third argument to 'open' is not an integer}} + if (fd > -1) + close(fd); +} -- 1.7.9.5
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
