mirror/gcc
2
0
Fork 0
mirror of git://gcc.gnu.org/git/gcc.git synced 2025-04-04 15:41:21 +08:00
Code Issues Packages Projects Releases Wiki Activity

analyzer: fix dedupe issue seen with CVE-2005-1689

Browse Source 
Whilst analyzing the reproducer for detecting CVE-2005-1689
(krb5-1.4.1's src/lib/krb5/krb/recvauth.c), the analyzer reported
11 double-free diagnostics on lines of the form:

   krb5_xfree(inbuf.data);

with no deduplication occcurring.

The root cause is that the diagnostics each have a COMPONENT_REF for
the inbuf.data, but they are different trees, and the de-duplication
logic was using pointer equality.

This patch replaces the pointer equality tests with calls to a new
pending_diagnostic::same_tree_p, implemented using simple_cst_equal.

With this patch, de-duplication occurs, and only 3 diagnostics are
reported.  The 11 diagnostics are partitioned into 3 dedupe keys,
2 with 2 duplicates and 1 with 7 duplicates.

gcc/analyzer/ChangeLog:
	* diagnostic-manager.cc (saved_diagnostic::operator==): Move here
	from header.  Replace pointer equality test on m_var with call to
	pending_diagnostic::same_tree_p.
	* diagnostic-manager.h (saved_diagnostic::operator==): Move to
	diagnostic-manager.cc.
	* pending-diagnostic.cc (pending_diagnostic::same_tree_p): New.
	* pending-diagnostic.h (pending_diagnostic::same_tree_p): New.
	* sm-file.cc (file_diagnostic::subclass_equal_p): Replace pointer
	equality on m_arg with call to pending_diagnostic::same_tree_p.
	* sm-malloc.cc (malloc_diagnostic::subclass_equal_p): Likewise.
	(possible_null_arg::subclass_equal_p): Likewise.
	(null_arg::subclass_equal_p): Likewise.
	(free_of_non_heap::subclass_equal_p): Likewise.
	* sm-pattern-test.cc (pattern_match::operator==): Likewise.
	* sm-sensitive.cc (exposure_through_output_file::operator==):
	Likewise.
	* sm-taint.cc (tainted_array_index::operator==): Likewise.

gcc/testsuite/ChangeLog:
	* gcc.dg/analyzer/CVE-2005-1689-dedupe-issue.c: New test.
This commit is contained in:
David Malcolm 2019-12-13 19:36:11 -05:00
parent 000c7a93bd
commit 14f9d7b9a7
12 changed files with 87 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
gcc/analyzer/ChangeLog
View File

@ -1,3 +1,23 @@
2020-01-14 David Malcolm <dmalcolm@redhat.com>
* diagnostic-manager.cc (saved_diagnostic::operator==): Move here
from header. Replace pointer equality test on m_var with call to
pending_diagnostic::same_tree_p.
* diagnostic-manager.h (saved_diagnostic::operator==): Move to
diagnostic-manager.cc.
* pending-diagnostic.cc (pending_diagnostic::same_tree_p): New.
* pending-diagnostic.h (pending_diagnostic::same_tree_p): New.
* sm-file.cc (file_diagnostic::subclass_equal_p): Replace pointer
equality on m_arg with call to pending_diagnostic::same_tree_p.
* sm-malloc.cc (malloc_diagnostic::subclass_equal_p): Likewise.
(possible_null_arg::subclass_equal_p): Likewise.
(null_arg::subclass_equal_p): Likewise.
(free_of_non_heap::subclass_equal_p): Likewise.
* sm-pattern-test.cc (pattern_match::operator==): Likewise.
* sm-sensitive.cc (exposure_through_output_file::operator==):
Likewise.
* sm-taint.cc (tainted_array_index::operator==): Likewise.
2020-01-14 David Malcolm <dmalcolm@redhat.com>
* diagnostic-manager.cc (dedupe_winners::add): Add logging

14
gcc/analyzer/diagnostic-manager.cc
View File

@ -91,6 +91,20 @@ saved_diagnostic::~saved_diagnostic ()
delete m_d;
}
bool
saved_diagnostic::operator== (const saved_diagnostic &other) const
{
return (m_sm == other.m_sm
/* We don't compare m_enode. */
&& m_snode == other.m_snode
&& m_stmt == other.m_stmt
/* We don't compare m_stmt_finder. */
&& pending_diagnostic::same_tree_p (m_var, other.m_var)
&& m_state == other.m_state
&& m_d->equal_p (*other.m_d)
&& m_trailing_eedge == other.m_trailing_eedge);
}
/* class diagnostic_manager. */
/* diagnostic_manager's ctor. */

13
gcc/analyzer/diagnostic-manager.h
View File

@ -34,18 +34,7 @@ public:
pending_diagnostic *d);
~saved_diagnostic ();
bool operator== (const saved_diagnostic &other) const
{
return (m_sm == other.m_sm
/* We don't compare m_enode. */
&& m_snode == other.m_snode
&& m_stmt == other.m_stmt
/* We don't compare m_stmt_finder. */
&& m_var == other.m_var
&& m_state == other.m_state
&& m_d->equal_p (*other.m_d)
&& m_trailing_eedge == other.m_trailing_eedge);
}
bool operator== (const saved_diagnostic &other) const;
//private:
const state_machine *m_sm;

9
gcc/analyzer/pending-diagnostic.cc
View File

@ -67,4 +67,13 @@ evdesc::event_desc::formatted_print (const char *fmt, ...) const
return result;
}
/* Return true if T1 and T2 are "the same" for the purposes of
diagnostic deduplication. */
bool
pending_diagnostic::same_tree_p (tree t1, tree t2)
{
return simple_cst_equal (t1, t2) == 1;
}
#endif /* #if ENABLE_ANALYZER */

4
gcc/analyzer/pending-diagnostic.h
View File

@ -169,6 +169,10 @@ class pending_diagnostic
below for a convenience subclass for implementing this. */
virtual bool subclass_equal_p (const pending_diagnostic &other) const = 0;
/* Return true if T1 and T2 are "the same" for the purposes of
diagnostic deduplication. */
static bool same_tree_p (tree t1, tree t2);
/* For greatest precision-of-wording, the various following "describe_*"
virtual functions give the pending diagnostic a way to describe events
in a diagnostic_path in terms that make sense for that diagnostic.

2
gcc/analyzer/sm-file.cc
View File

@ -96,7 +96,7 @@ public:
bool subclass_equal_p (const pending_diagnostic &base_other) const OVERRIDE
{
return m_arg == ((const file_diagnostic &)base_other).m_arg;
return same_tree_p (m_arg, ((const file_diagnostic &)base_other).m_arg);
}
label_text describe_state_change (const evdesc::state_change &change)

8
gcc/analyzer/sm-malloc.cc
View File

@ -102,7 +102,7 @@ public:
bool subclass_equal_p (const pending_diagnostic &base_other) const OVERRIDE
{
return m_arg == ((const malloc_diagnostic &)base_other).m_arg;
return same_tree_p (m_arg, ((const malloc_diagnostic &)base_other).m_arg);
}
label_text describe_state_change (const evdesc::state_change &change)
@ -282,7 +282,7 @@ public:
{
const possible_null_arg &sub_other
= (const possible_null_arg &)base_other;
return (m_arg == sub_other.m_arg
return (same_tree_p (m_arg, sub_other.m_arg)
&& m_fndecl == sub_other.m_fndecl
&& m_arg_idx == sub_other.m_arg_idx);
}
@ -373,7 +373,7 @@ public:
{
const null_arg &sub_other
= (const null_arg &)base_other;
return (m_arg == sub_other.m_arg
return (same_tree_p (m_arg, sub_other.m_arg)
&& m_fndecl == sub_other.m_fndecl
&& m_arg_idx == sub_other.m_arg_idx);
}
@ -499,7 +499,7 @@ public:
FINAL OVERRIDE
{
const free_of_non_heap &other = (const free_of_non_heap &)base_other;
return (m_arg == other.m_arg && m_kind == other.m_kind);
return (same_tree_p (m_arg, other.m_arg) && m_kind == other.m_kind);
}
bool emit (rich_location *rich_loc) FINAL OVERRIDE

4
gcc/analyzer/sm-pattern-test.cc
View File

@ -78,9 +78,9 @@ public:
bool operator== (const pattern_match &other) const
{
return (m_lhs == other.m_lhs
return (same_tree_p (m_lhs, other.m_lhs)
&& m_op == other.m_op
&& m_rhs == other.m_rhs);
&& same_tree_p (m_rhs, other.m_rhs));
}
bool emit (rich_location *rich_loc) FINAL OVERRIDE

2
gcc/analyzer/sm-sensitive.cc
View File

@ -95,7 +95,7 @@ public:
bool operator== (const exposure_through_output_file &other) const
{
return m_arg == other.m_arg;
return same_tree_p (m_arg, other.m_arg);
}
bool emit (rich_location *rich_loc) FINAL OVERRIDE

2
gcc/analyzer/sm-taint.cc
View File

@ -100,7 +100,7 @@ public:
bool operator== (const tainted_array_index &other) const
{
return m_arg == other.m_arg;
return same_tree_p (m_arg, other.m_arg);
}
bool emit (rich_location *rich_loc) FINAL OVERRIDE

4
gcc/testsuite/ChangeLog
View File

@ -1,3 +1,7 @@
2020-01-14 David Malcolm <dmalcolm@redhat.com>
* gcc.dg/analyzer/CVE-2005-1689-dedupe-issue.c: New test.
2020-01-15 Jakub Jelinek <jakub@redhat.com>
PR lto/91576

26
gcc/testsuite/gcc.dg/analyzer/CVE-2005-1689-dedupe-issue.c Normal file
View File

@ -0,0 +1,26 @@
#include <stdlib.h>
typedef struct _krb5_data {
char *data;
} krb5_data;
/* Ensure that we de-duplicate the various paths to reach here,
and only emit one diagnostic. */
void
recvauth_common(krb5_data inbuf)
{
free(inbuf.data);
free(inbuf.data); /* { dg-warning "double-'free'" } */
/* { dg-message "2 duplicates" "" { target *-*-* } .-1 } */
}
void krb5_recvauth(krb5_data inbuf)
{
recvauth_common(inbuf);
}
void krb5_recvauth_version(krb5_data inbuf)
{
recvauth_common(inbuf);
}