[pushed,5/6] analyzer: add kf_fopen

Message ID 20230822012127.2817996-5-dmalcolm@redhat.com
State Unresolved
Headers
Series [pushed,1/6] analyzer: convert note_adding_context to annotating_context |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

David Malcolm Aug. 22, 2023, 1:21 a.m. UTC
  Add checking to -fanalyzer that both params of calls to "fopen" are
valid null-terminated strings.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r14-3375-g4325c82736d9e8.

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.
---
 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
  

Patch

diff --git a/gcc/analyzer/kf.cc b/gcc/analyzer/kf.cc
index 6b2db8613768..1601cf15c685 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<kf_fopen> ());
     kfm.add ("putenv", make_unique<kf_putenv> ());
 
     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 000000000000..e5b00e93b6da
--- /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 } */
+}
+