@@ -214,6 +214,10 @@ Wanalyzer-tainted-size
Common Var(warn_analyzer_tainted_size) Init(1) Warning
Warn about code paths in which an unsanitized value is used as a size.
+Wanalyzer-unterminated-string
+Common Var(warn_analyzer_unterminated_string) Init(1) Warning
+Warn about code paths which attempt to find the length of an unterminated string.
+
Wanalyzer-use-after-free
Common Var(warn_analyzer_use_after_free) Init(1) Warning
Warn about code paths in which a freed value is used.
@@ -376,6 +376,13 @@ call_details::lookup_function_attribute (const char *attr_name) const
return lookup_attribute (attr_name, TYPE_ATTRIBUTES (allocfntype));
}
+void
+call_details::check_for_null_terminated_string_arg (unsigned arg_idx) const
+{
+ region_model *model = get_model ();
+ model->check_for_null_terminated_string_arg (*this, arg_idx);
+}
+
} // namespace ana
#endif /* #if ENABLE_ANALYZER */
@@ -71,6 +71,8 @@ public:
tree lookup_function_attribute (const char *attr_name) const;
+ void check_for_null_terminated_string_arg (unsigned arg_idx) const;
+
private:
const gcall *m_call;
region_model *m_model;
@@ -358,6 +358,22 @@ public:
}
};
+/* Handler for "__analyzer_get_strlen". */
+
+class kf_analyzer_get_strlen : public known_function
+{
+public:
+ bool matches_call_types_p (const call_details &cd) const final override
+ {
+ return cd.num_args () == 1 && cd.arg_is_pointer_p (0);
+ }
+ void impl_call_pre (const call_details &cd) const final override
+ {
+ cd.check_for_null_terminated_string_arg (0);
+ cd.set_any_lhs_with_defaults ();
+ }
+};
+
/* Populate KFM with instances of known functions used for debugging the
analyzer and for writing DejaGnu tests, all with a "__analyzer_" prefix. */
@@ -379,6 +395,8 @@ register_known_analyzer_functions (known_function_manager &kfm)
kfm.add ("__analyzer_eval", make_unique<kf_analyzer_eval> ());
kfm.add ("__analyzer_get_unknown_ptr",
make_unique<kf_analyzer_get_unknown_ptr> ());
+ kfm.add ("__analyzer_get_strlen",
+ make_unique<kf_analyzer_get_strlen> ());
}
} // namespace ana
@@ -414,6 +414,10 @@ kf_error::impl_call_pre (const call_details &cd) const
if (!model->add_constraint (status, EQ_EXPR, integer_zero_node, ctxt))
if (ctxt)
ctxt->terminate_path ();
+
+ /* Check "format" arg. */
+ const int fmt_arg_idx = (m_min_args == 3) ? 2 : 4;
+ model->check_for_null_terminated_string_arg (cd, fmt_arg_idx);
}
/* Handler for "free", after sm-handling.
@@ -674,6 +678,7 @@ public:
gcc_assert (fndecl);
region_model_context *ctxt = cd.get_ctxt ();
region_model *model = cd.get_model ();
+ model->check_for_null_terminated_string_arg (cd, 0);
const svalue *ptr_sval = cd.get_arg_svalue (0);
const region *reg
= model->deref_rvalue (ptr_sval, cd.get_arg_tree (0), ctxt);
@@ -949,6 +954,10 @@ public:
{
return (cd.num_args () == 2 && cd.arg_is_pointer_p (0));
}
+ void impl_call_pre (const call_details &cd) const final override
+ {
+ cd.check_for_null_terminated_string_arg (0);
+ }
void impl_call_post (const call_details &cd) const final override;
};
@@ -1109,6 +1118,7 @@ kf_strcpy::impl_call_pre (const call_details &cd) const
cd.get_ctxt ());
const svalue *src_contents_sval = model->get_store_value (src_reg,
cd.get_ctxt ());
+ cd.check_for_null_terminated_string_arg (1);
cd.maybe_set_lhs (dest_sval);
@@ -1136,6 +1146,7 @@ public:
{
region_model *model = cd.get_model ();
region_model_manager *mgr = cd.get_manager ();
+ cd.check_for_null_terminated_string_arg (0);
/* Ideally we'd get the size here, and simulate copying the bytes. */
const region *new_reg
= model->get_or_create_region_for_heap_alloc (NULL, cd.get_ctxt ());
@@ -3175,6 +3175,169 @@ region_model::set_value (tree lhs, tree rhs, region_model_context *ctxt)
set_value (lhs_reg, rhs_sval, ctxt);
}
+/* Look for the first 0 byte within STRING_CST.
+ If there is one, write its index to *OUT and return true.
+ Otherwise, return false. */
+
+static bool
+get_strlen (tree string_cst, int *out)
+{
+ gcc_assert (TREE_CODE (string_cst) == STRING_CST);
+
+ if (const void *p = memchr (TREE_STRING_POINTER (string_cst),
+ 0,
+ TREE_STRING_LENGTH (string_cst)))
+ {
+ *out = (const char *)p - TREE_STRING_POINTER (string_cst);
+ return true;
+ }
+ else
+ return false;
+}
+
+/* A bundle of information about a problematic argument at a callsite
+ for use by pending_diagnostic subclasses for reporting and
+ for deduplication. */
+
+struct call_arg_details
+{
+public:
+ call_arg_details (const call_details &cd, unsigned arg_idx)
+ : m_call (cd.get_call_stmt ()),
+ m_called_fndecl (cd.get_fndecl_for_call ()),
+ m_arg_idx (arg_idx),
+ m_arg_expr (cd.get_arg_tree (arg_idx))
+ {
+ }
+
+ bool operator== (const call_arg_details &other) const
+ {
+ return (m_call == other.m_call
+ && m_called_fndecl == other.m_called_fndecl
+ && m_arg_idx == other.m_arg_idx
+ && pending_diagnostic::same_tree_p (m_arg_expr, other.m_arg_expr));
+ }
+
+ const gcall *m_call;
+ tree m_called_fndecl;
+ unsigned m_arg_idx; // 0-based
+ tree m_arg_expr;
+};
+
+/* Issue a note specifying that a particular function parameter is expected
+ to be a valid null-terminated string. */
+
+static void
+inform_about_expected_null_terminated_string_arg (const call_arg_details &ad)
+{
+ // TODO: ideally we'd underline the param here
+ inform (DECL_SOURCE_LOCATION (ad.m_called_fndecl),
+ "argument %d of %qD must be a pointer to a null-terminated string",
+ ad.m_arg_idx + 1, ad.m_called_fndecl);
+}
+
+/* A subclass of pending_diagnostic for complaining about uses
+ of unterminated strings (thus accessing beyond the bounds
+ of a buffer). */
+
+class unterminated_string_arg
+: public pending_diagnostic_subclass<unterminated_string_arg>
+{
+public:
+ unterminated_string_arg (const call_arg_details arg_details)
+ : m_arg_details (arg_details)
+ {
+ gcc_assert (m_arg_details.m_called_fndecl);
+ }
+
+ const char *get_kind () const final override
+ {
+ return "unterminated_string_arg";
+ }
+
+ bool operator== (const unterminated_string_arg &other) const
+ {
+ return m_arg_details == other.m_arg_details;
+ }
+
+ int get_controlling_option () const final override
+ {
+ return OPT_Wanalyzer_unterminated_string;
+ }
+
+ bool emit (rich_location *rich_loc, logger *) final override
+ {
+ auto_diagnostic_group d;
+ bool warned;
+ if (m_arg_details.m_arg_expr)
+ warned = warning_at (rich_loc, get_controlling_option (),
+ "passing pointer to unterminated string %qE"
+ " as argument %i of %qE",
+ m_arg_details.m_arg_expr,
+ m_arg_details.m_arg_idx + 1,
+ m_arg_details.m_called_fndecl);
+ else
+ warned = warning_at (rich_loc, get_controlling_option (),
+ "passing pointer to unterminated string"
+ " as argument %i of %qE",
+ m_arg_details.m_arg_idx + 1,
+ m_arg_details.m_called_fndecl);
+ if (warned)
+ inform_about_expected_null_terminated_string_arg (m_arg_details);
+ return warned;
+ }
+
+ label_text describe_final_event (const evdesc::final_event &ev) final override
+ {
+ return ev.formatted_print
+ ("passing pointer to unterminated buffer as argument %i of %qE"
+ " would lead to read past the end of the buffer",
+ m_arg_details.m_arg_idx + 1,
+ m_arg_details.m_called_fndecl);
+ }
+
+private:
+ const call_arg_details m_arg_details;
+};
+
+/* Check that argument ARG_IDX (0-based) to the call described by CD
+ is a pointer to a valid null-terminated string.
+
+ Complain if the buffer pointed to isn't null-terminated.
+
+ TODO: we should also complain if:
+ - the pointer is NULL (or could be)
+ - the buffer pointed to is uninitalized before any 0-terminator
+ - the 0-terminator is within the bounds of the underlying base region
+
+ We're checking that the called function could validly iterate through
+ the buffer reading it until it finds a 0 byte (such as by calling
+ strlen, or equivalent code). */
+
+void
+region_model::check_for_null_terminated_string_arg (const call_details &cd,
+ unsigned arg_idx)
+{
+ region_model_context *ctxt = cd.get_ctxt ();
+
+ const svalue *arg_sval = cd.get_arg_svalue (arg_idx);
+ const region *buf_reg
+ = deref_rvalue (arg_sval, cd.get_arg_tree (arg_idx), ctxt);
+
+ const svalue *contents_sval = get_store_value (buf_reg, ctxt);
+
+ if (tree cst = contents_sval->maybe_get_constant ())
+ if (TREE_CODE (cst) == STRING_CST)
+ {
+ int cst_strlen;
+ if (!get_strlen (cst, &cst_strlen))
+ {
+ call_arg_details arg_details (cd, arg_idx);
+ ctxt->warn (make_unique<unterminated_string_arg> (arg_details));
+ }
+ }
+}
+
/* Remove all bindings overlapping REG within the store. */
void
@@ -502,6 +502,9 @@ class region_model
const svalue *sval_hint,
region_model_context *ctxt) const;
+ void check_for_null_terminated_string_arg (const call_details &cd,
+ unsigned idx);
+
private:
const region *get_lvalue_1 (path_var pv, region_model_context *ctxt) const;
const svalue *get_rvalue_1 (path_var pv, region_model_context *ctxt) const;
@@ -652,6 +652,14 @@ __analyzer_get_unknown_ptr ();
@end smallexample
will obtain an unknown @code{void *}.
+@item __analyzer_get_strlen
+@smallexample
+__analyzer_get_strlen (buf);
+@end smallexample
+will emit a warning if PTR doesn't point to a null-terminated string.
+TODO: eventually get the strlen of the buffer (without the
+optimizer touching it).
+
@end table
@subsection Other Debugging Techniques
@@ -486,6 +486,7 @@ Objective-C and Objective-C++ Dialects}.
-Wno-analyzer-tainted-size
-Wanalyzer-too-complex
-Wno-analyzer-unsafe-call-within-signal-handler
+-Wno-analyzer-unterminated-string
-Wno-analyzer-use-after-free
-Wno-analyzer-use-of-pointer-in-stale-stack-frame
-Wno-analyzer-use-of-uninitialized-value
@@ -10318,6 +10319,7 @@ Enabling this option effectively enables the following warnings:
-Wanalyzer-shift-count-overflow
-Wanalyzer-stale-setjmp-buffer
-Wanalyzer-unsafe-call-within-signal-handler
+-Wanalyzer-unterminated-string
-Wanalyzer-use-after-free
-Wanalyzer-use-of-pointer-in-stale-stack-frame
-Wanalyzer-use-of-uninitialized-value
@@ -10907,6 +10909,17 @@ called from a signal handler.
See @uref{https://cwe.mitre.org/data/definitions/479.html, CWE-479: Signal Handler Use of a Non-reentrant Function}.
+@opindex Wanalyzer-unterminated-string
+@opindex Wno-analyzer-unterminated-string
+@item -Wno-analyzer-unterminated-string
+This warning requires @option{-fanalyzer}, which enables it; use
+@option{-Wno-analyzer-unterminated-string} to disable it.
+
+This diagnostic warns about code paths which attempt to find the length
+of an unterminated string. For example, passing a pointer to an unterminated
+buffer to @code{strlen} would lead to accesses beyond the end of the buffer
+whilst attempting to find the terminating zero character.
+
@opindex Wanalyzer-use-after-free
@opindex Wno-analyzer-use-after-free
@item -Wno-analyzer-use-after-free
@@ -53,4 +53,9 @@ extern void __analyzer_eval (int);
/* Obtain an "unknown" void *. */
extern void *__analyzer_get_unknown_ptr (void);
+/* Complain if PTR doesn't point to a null-terminated string.
+ TODO: eventually get the strlen of the buffer (without the
+ optimizer touching it). */
+extern __SIZE_TYPE__ __analyzer_get_strlen (const char *ptr);
+
#endif /* #ifndef ANALYZER_DECLS_H. */
@@ -64,3 +64,15 @@ void test_6 (int st, const char *str)
error (st, errno, "test: %s", str);
__analyzer_eval (st == 0); /* { dg-warning "TRUE" } */
}
+
+char *test_error_unterminated (int st)
+{
+ char fmt[3] = "abc";
+ error (st, errno, fmt); /* { dg-warning "passing pointer to unterminated string '&fmt' as argument 3 of 'error'" } */
+}
+
+char *test_error_at_line_unterminated (int st, int errno)
+{
+ char fmt[3] = "abc";
+ error_at_line (st, errno, __FILE__, __LINE__, fmt); /* { dg-warning "passing pointer to unterminated string '&fmt' as argument 5 of 'error_at_line'" } */
+}
new file mode 100644
@@ -0,0 +1,30 @@
+#include "analyzer-decls.h"
+
+#define NULL ((void *)0)
+typedef __SIZE_TYPE__ size_t;
+
+void test_terminated (void)
+{
+ __analyzer_get_strlen ("abc"); /* { dg-bogus "" } */
+}
+
+void test_unterminated (void)
+{
+ char buf[3] = "abc";
+ __analyzer_get_strlen (buf); /* { dg-warning "passing pointer to unterminated string '&buf' as argument 1 of '__analyzer_get_strlen'" } */
+}
+
+void test_embedded_nul (void)
+{
+ char buf[3] = "a\0c";
+ __analyzer_get_strlen (buf); /* { dg-bogus "" } */
+}
+
+void test_fully_initialized_but_unterminated (void)
+{
+ char buf[3];
+ buf[0] = 'a';
+ buf[1] = 'b';
+ buf[2] = 'c';
+ __analyzer_get_strlen (buf); /* { dg-warning "passing pointer to unterminated string '&buf' as argument 1 of '__analyzer_get_strlen'" "" { xfail *-*-* } } */
+}
@@ -108,3 +108,10 @@ void test_outer (void)
char arr_outer[] = "NAME=VALUE"; /* { dg-message "'arr_outer' declared on stack here" } */
__analyzer_test_inner (arr_outer);
}
+
+void test_unterminated (void)
+{
+ char buf[3] = "abc";
+ putenv (buf); /* { dg-warning "passing pointer to unterminated string" } */
+ /* { dg-warning "'putenv' on a pointer to automatic variable 'buf'" "POS34-C" { target *-*-* } .-1 } */
+}
@@ -25,3 +25,9 @@ void test_3 (const char *s, int c)
char *p = strchr (s, c); /* { dg-message "when 'strchr' returns NULL"} */
*p = 'A'; /* { dg-warning "dereference of NULL 'p'" "null deref" } */
}
+
+void test_unterminated (int c)
+{
+ char buf[3] = "abc";
+ strchr (buf, c); /* { dg-warning "passing pointer to unterminated string '&buf' as argument 1 of 'strchr'" } */
+}
@@ -16,3 +16,9 @@ test_1a (char *dst, char *src)
__analyzer_eval (result == dst); /* { dg-warning "TRUE" } */
return result;
}
+
+char *test_unterminated (char *dst)
+{
+ char buf[3] = "abc";
+ return strcpy (dst, buf); /* { dg-warning "passing pointer to unterminated string '&buf' as argument 2 of 'strcpy'" } */
+}
@@ -38,3 +38,9 @@ void test_6 (const char *s)
char *p = __builtin_strdup (s); /* { dg-message "this call could return NULL" } */
requires_nonnull (p); /* { dg-warning "use of possibly-NULL 'p'" } */
}
+
+char *test_unterminated (void)
+{
+ char buf[3] = "abc";
+ return strdup (buf); /* { dg-warning "passing pointer to unterminated string '&buf' as argument 1 of 'strdup'" } */
+}