From 4325c82736d9e8a14b312fd1558e2788b69278cd Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Mon, 21 Aug 2023 21:13:19 -0400 Subject: [PATCH] analyzer: add kf_fopen Add checking to -fanalyzer that both params of calls to "fopen" are valid null-terminated strings. gcc/analyzer/ChangeLog: * kf.cc (class kf_fopen): New. (register_known_functions): Register it. gcc/testsuite/ChangeLog: * gcc.dg/analyzer/fopen-1.c: New test. Signed-off-by: David Malcolm --- gcc/analyzer/kf.cc | 28 +++++++++++ gcc/testsuite/gcc.dg/analyzer/fopen-1.c | 66 +++++++++++++++++++++++++ 2 files changed, 94 insertions(+) create mode 100644 gcc/testsuite/gcc.dg/analyzer/fopen-1.c diff --git a/gcc/analyzer/kf.cc b/gcc/analyzer/kf.cc index 6b2db861376..1601cf15c68 100644 --- a/gcc/analyzer/kf.cc +++ b/gcc/analyzer/kf.cc @@ -420,6 +420,33 @@ kf_error::impl_call_pre (const call_details &cd) const model->check_for_null_terminated_string_arg (cd, fmt_arg_idx); } +/* Handler for fopen. + FILE *fopen (const char *filename, const char *mode); + See e.g. https://en.cppreference.com/w/c/io/fopen + https://www.man7.org/linux/man-pages/man3/fopen.3.html + https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/fopen-wfopen?view=msvc-170 */ + +class kf_fopen : public known_function +{ +public: + bool matches_call_types_p (const call_details &cd) const final override + { + return (cd.num_args () == 2 + && cd.arg_is_pointer_p (0) + && cd.arg_is_pointer_p (1)); + } + + void impl_call_pre (const call_details &cd) const final override + { + cd.check_for_null_terminated_string_arg (0); + cd.check_for_null_terminated_string_arg (1); + cd.set_any_lhs_with_defaults (); + + /* fopen's mode param is effectively a mini-DSL, but there are various + non-standard extensions, so we don't bother to check it. */ + } +}; + /* Handler for "free", after sm-handling. If the ptr points to an underlying heap region, delete the region, @@ -1422,6 +1449,7 @@ register_known_functions (known_function_manager &kfm) /* Known POSIX functions, and some non-standard extensions. */ { + kfm.add ("fopen", make_unique ()); kfm.add ("putenv", make_unique ()); register_known_fd_functions (kfm); diff --git a/gcc/testsuite/gcc.dg/analyzer/fopen-1.c b/gcc/testsuite/gcc.dg/analyzer/fopen-1.c new file mode 100644 index 00000000000..e5b00e93b6d --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/fopen-1.c @@ -0,0 +1,66 @@ +typedef struct FILE FILE; +FILE *fopen (const char *pathname, const char *mode); +#define NULL ((void *)0) + +FILE * +test_passthrough (const char *pathname, const char *mode) +{ + return fopen (pathname, mode); +} + +FILE * +test_null_pathname (const char *pathname, const char *mode) +{ + return fopen (NULL, mode); +} + +FILE * +test_null_mode (const char *pathname) +{ + return fopen (pathname, NULL); +} + +FILE * +test_simple_r (void) +{ + return fopen ("foo.txt", "r"); +} + +FILE * +test_swapped_args (void) +{ + return fopen ("r", "foo.txt"); /* TODO: would be nice to detect this. */ +} + +FILE * +test_unterminated_pathname (const char *mode) +{ + char buf[3] = "abc"; + return fopen (buf, mode); /* { dg-warning "stack-based buffer over-read" } */ + /* { dg-message "while looking for null terminator for argument 1 \\('&buf'\\) of 'fopen'..." "event" { target *-*-* } .-1 } */ +} + +FILE * +test_unterminated_mode (const char *filename) +{ + char buf[3] = "abc"; + return fopen (filename, buf); /* { dg-warning "stack-based buffer over-read" } */ + /* { dg-message "while looking for null terminator for argument 2 \\('&buf'\\) of 'fopen'..." "event" { target *-*-* } .-1 } */ +} + +FILE * +test_uninitialized_pathname (const char *mode) +{ + char buf[10]; + return fopen (buf, mode); /* { dg-warning "use of uninitialized value 'buf\\\[0\\\]'" } */ + /* { dg-message "while looking for null terminator for argument 1 \\('&buf'\\) of 'fopen'..." "event" { target *-*-* } .-1 } */ +} + +FILE * +test_uninitialized_mode (const char *filename) +{ + char buf[10]; + return fopen (filename, buf); /* { dg-warning "use of uninitialized value 'buf\\\[0\\\]'" } */ + /* { dg-message "while looking for null terminator for argument 2 \\('&buf'\\) of 'fopen'..." "event" { target *-*-* } .-1 } */ +} +