From patchwork Sat Nov 13 20:37:27 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 47619 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 0626D3858410 for ; Sat, 13 Nov 2021 20:38:14 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 0626D3858410 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1636835894; bh=f4T7gGs0/F73buSzDODMeMiO1sj9UJsLvOu0IwdWUiQ=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=g+xM0QZ8NPCh4Hqrj6v+f0YZO0xSb5IPU6rE6MtHfVaU/NQZ3FavLVGsLeUk2BN01 3CSw+0wZVPdXGNMGU/cKb6N1GpIwCzs0mp6dyo04QTNUVPrejQFdjmmCe0bK98raTv APq/TJHJv0Gi1yCS2Q8GdRSueRlxhWNh8UgdgPFo= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTPS id E98AC3858410 for ; Sat, 13 Nov 2021 20:37:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E98AC3858410 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-591-5Y027zXeNIiaTJImO0XgQg-1; Sat, 13 Nov 2021 15:37:42 -0500 X-MC-Unique: 5Y027zXeNIiaTJImO0XgQg-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 6CA5415720; Sat, 13 Nov 2021 20:37:41 +0000 (UTC) Received: from t14s.localdomain.com (ovpn-113-54.phx2.redhat.com [10.3.113.54]) by smtp.corp.redhat.com (Postfix) with ESMTP id F015E19723; Sat, 13 Nov 2021 20:37:40 +0000 (UTC) To: gcc-patches@gcc.gnu.org, linux-toolchains@vger.kernel.org Subject: [PATCH 2/6] Add returns_zero_on_success/failure attributes Date: Sat, 13 Nov 2021 15:37:27 -0500 Message-Id: <20211113203732.2098220-4-dmalcolm@redhat.com> In-Reply-To: <20211113203732.2098220-1-dmalcolm@redhat.com> References: <20211113203732.2098220-1-dmalcolm@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-13.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: David Malcolm via Gcc-patches From: David Malcolm Reply-To: David Malcolm Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" This patch adds two new attributes. The followup patch makes use of the attributes in -fanalyzer. gcc/c-family/ChangeLog: * c-attribs.c (attr_noreturn_exclusions): Add "returns_zero_on_failure" and "returns_zero_on_success". (attr_returns_twice_exclusions): Likewise. (attr_returns_zero_on_exclusions): New. (c_common_attribute_table): Add "returns_zero_on_failure" and "returns_zero_on_success". (handle_returns_zero_on_attributes): New. gcc/ChangeLog: * doc/extend.texi (Common Function Attributes): Document "returns_zero_on_failure" and "returns_zero_on_success". gcc/testsuite/ChangeLog: * c-c++-common/attr-returns-zero-on-1.c: New test. Signed-off-by: David Malcolm --- gcc/c-family/c-attribs.c | 37 ++++++++++ gcc/doc/extend.texi | 16 +++++ .../c-c++-common/attr-returns-zero-on-1.c | 68 +++++++++++++++++++ 3 files changed, 121 insertions(+) create mode 100644 gcc/testsuite/c-c++-common/attr-returns-zero-on-1.c diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c index 100c2dabab2..9e03156de5e 100644 --- a/gcc/c-family/c-attribs.c +++ b/gcc/c-family/c-attribs.c @@ -153,6 +153,7 @@ static tree handle_argspec_attribute (tree *, tree, tree, int, bool *); static tree handle_fnspec_attribute (tree *, tree, tree, int, bool *); static tree handle_warn_unused_attribute (tree *, tree, tree, int, bool *); static tree handle_returns_nonnull_attribute (tree *, tree, tree, int, bool *); +static tree handle_returns_zero_on_attributes (tree *, tree, tree, int, bool *); static tree handle_omp_declare_simd_attribute (tree *, tree, tree, int, bool *); static tree handle_omp_declare_variant_attribute (tree *, tree, tree, int, @@ -221,6 +222,8 @@ extern const struct attribute_spec::exclusions attr_noreturn_exclusions[] = ATTR_EXCL ("pure", true, true, true), ATTR_EXCL ("returns_twice", true, true, true), ATTR_EXCL ("warn_unused_result", true, true, true), + ATTR_EXCL ("returns_zero_on_failure", true, true, true), + ATTR_EXCL ("returns_zero_on_success", true, true, true), ATTR_EXCL (NULL, false, false, false), }; @@ -235,6 +238,8 @@ attr_warn_unused_result_exclusions[] = static const struct attribute_spec::exclusions attr_returns_twice_exclusions[] = { ATTR_EXCL ("noreturn", true, true, true), + ATTR_EXCL ("returns_zero_on_failure", true, true, true), + ATTR_EXCL ("returns_zero_on_success", true, true, true), ATTR_EXCL (NULL, false, false, false), }; @@ -275,6 +280,16 @@ static const struct attribute_spec::exclusions attr_stack_protect_exclusions[] = ATTR_EXCL (NULL, false, false, false), }; +/* Exclusions that apply to the returns_zero_on_* attributes. */ +static const struct attribute_spec::exclusions + attr_returns_zero_on_exclusions[] = +{ + ATTR_EXCL ("noreturn", true, true, true), + ATTR_EXCL ("returns_twice", true, true, true), + ATTR_EXCL ("returns_zero_on_failure", true, true, true), + ATTR_EXCL ("returns_zero_on_success", true, true, true), + ATTR_EXCL (NULL, false, false, false), +}; /* Table of machine-independent attributes common to all C-like languages. @@ -493,6 +508,12 @@ const struct attribute_spec c_common_attribute_table[] = handle_warn_unused_attribute, NULL }, { "returns_nonnull", 0, 0, false, true, true, false, handle_returns_nonnull_attribute, NULL }, + { "returns_zero_on_failure",0, 0, false, true, true, false, + handle_returns_zero_on_attributes, + attr_returns_zero_on_exclusions }, + { "returns_zero_on_success",0, 0, false, true, true, false, + handle_returns_zero_on_attributes, + attr_returns_zero_on_exclusions }, { "omp declare simd", 0, -1, true, false, false, false, handle_omp_declare_simd_attribute, NULL }, { "omp declare variant base", 0, -1, true, false, false, false, @@ -5660,6 +5681,22 @@ handle_returns_nonnull_attribute (tree *node, tree name, tree, int, return NULL_TREE; } +/* Handle "returns_zero_on_failure" and "returns_zero_on_success" attributes; + arguments as in struct attribute_spec.handler. */ + +static tree +handle_returns_zero_on_attributes (tree *node, tree name, tree, int, + bool *no_add_attrs) +{ + if (!INTEGRAL_TYPE_P (TREE_TYPE (*node))) + { + error ("%qE attribute on a function not returning an integral type", + name); + *no_add_attrs = true; + } + return NULL_TREE; +} + /* Handle a "designated_init" attribute; arguments as in struct attribute_spec.handler. */ diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index e9f47519df2..5a6ef464779 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -3784,6 +3784,22 @@ function. Examples of such functions are @code{setjmp} and @code{vfork}. The @code{longjmp}-like counterpart of such function, if any, might need to be marked with the @code{noreturn} attribute. +@item returns_zero_on_failure +@cindex @code{returns_zero_on_failure} function attribute +The @code{returns_zero_on_failure} attribute hints that the function +can succeed or fail, returning non-zero on success and zero on failure. +This is used by the @option{-fanalyzer} option to consider both outcomes +separately, which may improve how it explores error-handling paths, and +how such outcomes are labelled in diagnostics. It is also a hint +to the human reader of the source code. + +@item returns_zero_on_success +@cindex @code{returns_zero_on_success} function attribute +The @code{returns_zero_on_success} attribute is identical to the +@code{returns_zero_on_failure} attribute, apart from having the +opposite interpretation of the return value: zero on success, non-zero +on failure. + @item section ("@var{section-name}") @cindex @code{section} function attribute @cindex functions in arbitrary sections diff --git a/gcc/testsuite/c-c++-common/attr-returns-zero-on-1.c b/gcc/testsuite/c-c++-common/attr-returns-zero-on-1.c new file mode 100644 index 00000000000..5475dfe61db --- /dev/null +++ b/gcc/testsuite/c-c++-common/attr-returns-zero-on-1.c @@ -0,0 +1,68 @@ +/* Verify the parsing of the "returns_zero_on_{sucess|failure}" attributes. */ + +/* Correct usage. */ + +extern int test_int_return_s () + __attribute__((returns_zero_on_success)); +extern long test_long_return_f () + __attribute__((returns_zero_on_failure)); + +/* Should complain if not a function. */ + +extern int not_a_function_s + __attribute__((returns_zero_on_success)); /* { dg-warning "'returns_zero_on_success' attribute only applies to function types" } */ +extern int not_a_function_f + __attribute__((returns_zero_on_failure)); /* { dg-warning "'returns_zero_on_failure' attribute only applies to function types" } */ + +/* Should complain if return type is not integral. */ + +extern void test_void_return_s () + __attribute__((returns_zero_on_success)); /* { dg-error "'returns_zero_on_success' attribute on a function not returning an integral type" } */ +extern void test_void_return_f () + __attribute__((returns_zero_on_failure)); /* { dg-error "'returns_zero_on_failure' attribute on a function not returning an integral type" } */ + +extern void *test_void_star_return_s () + __attribute__((returns_zero_on_success)); /* { dg-error "'returns_zero_on_success' attribute on a function not returning an integral type" } */ +extern void *test_void_star_return_f () + __attribute__((returns_zero_on_failure)); /* { dg-error "'returns_zero_on_failure' attribute on a function not returning an integral type" } */ + +/* (and this prevents mixing with returns_non_null, which requires a pointer). */ + +/* Should complain if more than one returns_* attribute. */ + +extern int test_void_returns_s_f () + __attribute__((returns_zero_on_success)) + __attribute__((returns_zero_on_failure)); /* { dg-warning "ignoring attribute 'returns_zero_on_failure' because it conflicts with attribute 'returns_zero_on_success'" } */ +extern int test_void_returns_f_s () + __attribute__((returns_zero_on_failure)) + __attribute__((returns_zero_on_success)); /* { dg-warning "ignoring attribute 'returns_zero_on_success' because it conflicts with attribute 'returns_zero_on_failure'" } */ + +/* Should complain if mixed with "noreturn". */ + +extern int test_noreturn_returns_s () + __attribute__((noreturn)) + __attribute__((returns_zero_on_success)); /* { dg-warning "ignoring attribute 'returns_zero_on_success' because it conflicts with attribute 'noreturn'" } */ +extern int test_returns_s_noreturn () + __attribute__((returns_zero_on_success)) + __attribute__((noreturn)); /* { dg-warning "ignoring attribute 'noreturn' because it conflicts with attribute 'returns_zero_on_success'" } */ +extern int test_noreturn_returns_f () + __attribute__((noreturn)) + __attribute__((returns_zero_on_failure)); /* { dg-warning "ignoring attribute 'returns_zero_on_failure' because it conflicts with attribute 'noreturn'" } */ +extern int test_returns_f_noreturn () + __attribute__((returns_zero_on_failure)) + __attribute__((noreturn)); /* { dg-warning "ignoring attribute 'noreturn' because it conflicts with attribute 'returns_zero_on_failure'" } */ + +/* Should complain if mixed with "returns_twice". */ + +extern int test_returns_twice_returns_s () + __attribute__((returns_twice)) + __attribute__((returns_zero_on_success)); /* { dg-warning "ignoring attribute 'returns_zero_on_success' because it conflicts with attribute 'returns_twice'" } */ +extern int test_returns_s_returns_twice () + __attribute__((returns_zero_on_success)) + __attribute__((returns_twice)); /* { dg-warning "ignoring attribute 'returns_twice' because it conflicts with attribute 'returns_zero_on_success'" } */ +extern int test_returns_twice_returns_f () + __attribute__((returns_twice)) + __attribute__((returns_zero_on_failure)); /* { dg-warning "ignoring attribute 'returns_zero_on_failure' because it conflicts with attribute 'returns_twice'" } */ +extern int test_returns_f_returns_twice () + __attribute__((returns_zero_on_failure)) + __attribute__((returns_twice)); /* { dg-warning "ignoring attribute 'returns_twice' because it conflicts with attribute 'returns_zero_on_failure'" } */ From patchwork Sat Nov 13 20:37:28 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 47627 X-Patchwork-Delegate: dmalcolm@redhat.com Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1CED83858033 for ; Sat, 13 Nov 2021 20:46:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1CED83858033 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1636836394; bh=XUdW5QyEG8MuzfQx2bDIorvziH8Cxu/sbT5HHEfTtMs=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=QBvTQLGd9p5Zsi4JNHVfC3tRBChtFnDLXb5CLFlhNGUfAuMnHFskZK5Pf3WrOkfi6 tV8KxxASZL65INF+x8+00HHp4LqezCFcJcmo+C/yFHiLUsj54fUA1T7relcmVgQSbI 8+rrWfp4Z2jKt47cNsz7albUSoSNPe8cxv6V0fg8= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTPS id 319463857C47 for ; Sat, 13 Nov 2021 20:37:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 319463857C47 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-527-9-UC6jWyOjGY3qYkvtJm7w-1; Sat, 13 Nov 2021 15:37:43 -0500 X-MC-Unique: 9-UC6jWyOjGY3qYkvtJm7w-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 3D66A1006AA0; Sat, 13 Nov 2021 20:37:42 +0000 (UTC) Received: from t14s.localdomain.com (ovpn-113-54.phx2.redhat.com [10.3.113.54]) by smtp.corp.redhat.com (Postfix) with ESMTP id 98C8219D9D; Sat, 13 Nov 2021 20:37:41 +0000 (UTC) To: gcc-patches@gcc.gnu.org, linux-toolchains@vger.kernel.org Subject: [PATCH 3/6] analyzer: implement infoleak detection Date: Sat, 13 Nov 2021 15:37:28 -0500 Message-Id: <20211113203732.2098220-5-dmalcolm@redhat.com> In-Reply-To: <20211113203732.2098220-1-dmalcolm@redhat.com> References: <20211113203732.2098220-1-dmalcolm@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-13.0 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_ASCII_DIVIDERS, KAM_SHORT, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: David Malcolm via Gcc-patches From: David Malcolm Reply-To: David Malcolm Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" This patch adds a new -Wanalyzer-exposure-through-uninit-copy, emitted by -fanalyzer if it detects copying of uninitialized data through a pointer to an untrusted region. The patch uses region::untrusted_p in the analyzer and __user in the testsuite to identify untrusted regions, but the implementation of this is left to follow-up patches, so that they can be either via the custom_address_space pragma, or via __attribute__((untrusted). The diagnostic uses notes to express what fields and padding within a struct have not been initialized. For example: infoleak-CVE-2011-1078-2.c: In function ‘test_1’: infoleak-CVE-2011-1078-2.c:28:9: warning: potential exposure of sensitive information by copying uninitialized data from stack across trust boundary [CWE-200] [-Wanalyzer-exposure-through-uninit-copy] 28 | copy_to_user(optval, &cinfo, sizeof(cinfo)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ‘test_1’: events 1-3 | | 21 | struct sco_conninfo cinfo; | | ^~~~~ | | | | | (1) region created on stack here | | (2) capacity: 6 bytes |...... | 28 | copy_to_user(optval, &cinfo, sizeof(cinfo)); | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | | | (3) uninitialized data copied from stack here | infoleak-CVE-2011-1078-2.c:28:9: note: 1 byte is uninitialized 28 | copy_to_user(optval, &cinfo, sizeof(cinfo)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ infoleak-CVE-2011-1078-2.c:14:15: note: padding after field ‘dev_class’ is uninitialized (1 byte) 14 | __u8 dev_class[3]; | ^~~~~~~~~ infoleak-CVE-2011-1078-2.c:21:29: note: suggest forcing zero-initialization by providing a ‘{0}’ initializer 21 | struct sco_conninfo cinfo; | ^~~~~ | = {0} gcc/ChangeLog: * Makefile.in (ANALYZER_OBJS): Add analyzer/trust-boundaries.o. * doc/invoke.texi (-Wanalyzer-exposure-through-uninit-copy): New. gcc/analyzer/ChangeLog: * analyzer.opt (Wanalyzer-exposure-through-uninit-copy): New. * checker-path.cc (event_kind_to_string): Handle EK_REGION_CREATION. (region_creation_event::region_creation_event): New. (region_creation_event::get_desc): New. (checker_path::add_region_creation_events): New. * checker-path.h (enum event_kind): Add EK_REGION_CREATION. (enum rce_kind): New. (class region_creation_event): New. (checker_path::add_region_creation_events): New. * diagnostic-manager.cc (diagnostic_manager::emit_saved_diagnostic): Pass NULL to add_events_for_eedge. (diagnostic_manager::build_emission_path): Create interesting_t instance and pass it to mark_interesting_stuff, and to add_events_for_eedge. (diagnostic_manager::add_events_for_eedge): Add "interest" param. Use it to create region_creation_events for on-stack regions when a stack frame is pushed and for heap-based and alloca regions. (diagnostic_manager::prune_for_sm_diagnostic): Handle EK_REGION_CREATION. * diagnostic-manager.h (diagnostic_manager::add_events_for_eedge): Add "interest" param. * pending-diagnostic.cc: Include "selftest.h", "tristate.h", "analyzer/call-string.h", "analyzer/program-point.h", "analyzer/store.h", and "analyzer/region-model.h". (interesting_t::add_region_creation): New. (interesting_t::dump_to_pp): New. * pending-diagnostic.h (struct interesting_t): New. (pending_diagnostic::mark_interesting_stuff): New. * region-model-impl-calls.cc (call_details::get_logger): New. * region-model.cc: Include "analyzer/call-info.h". (enum return_meaning): New. (get_return_meaning): New. (region_model::update_for_zero_return): New. (region_model::update_for_nonzero_return): New. (class maybe_returns_zero_call_info): New. (class copy_success): New. (class copy_failure): New. (maybe_simplify_upper_bound): New. (region_model::maybe_get_copy_bounds): New. (struct copy_fn_details): New. (is_copy_function): New. (region_model::handle_copy_function): New. (region_model::on_call_pre): Call is_copy_function and handle_copy_function. (region_model::set_value): Add param "src_reg". Call maybe_complain_about_infoleak for copies to untrusted regions. * region-model.h (call_details::get_logger): New. (struct copy_fn_details): New forward decl. (region_model::handle_copy_function): New. (region_model::maybe_get_copy_bounds): New. (region_model::update_for_zero_return): New. (region_model::update_for_nonzero_return): New. (region_model::set_value): Add param "src_reg". (region_model::maybe_complain_about_infoleak): New. * region.cc (region::get_addr_space): New. * region.h (region::get_addr_space): New. (region::untrusted_p): New. (frame_region::get_fndecl): New. * store.h (binding_cluster::get_base_region): New. * trust-boundaries.cc: New file. gcc/testsuite/ChangeLog: * gcc.dg/analyzer/copy-function-1.c: New test. * gcc.dg/analyzer/copy_from_user-1.c: New test. * gcc.dg/analyzer/infoleak-1.c: New test. * gcc.dg/analyzer/infoleak-2.c: New test. * gcc.dg/analyzer/infoleak-3.c: New test. * gcc.dg/analyzer/infoleak-5.c: New test. * gcc.dg/analyzer/infoleak-CVE-2011-1078-1.c: New test. * gcc.dg/analyzer/infoleak-CVE-2011-1078-2.c: New test. * gcc.dg/analyzer/infoleak-CVE-2014-1446-1.c: New test. * gcc.dg/analyzer/infoleak-CVE-2017-18549-1.c: New test. * gcc.dg/analyzer/infoleak-CVE-2017-18550-1.c: New test. * gcc.dg/analyzer/infoleak-antipatterns-1.c: New test. * gcc.dg/analyzer/infoleak-fixit-1.c: New test. * gcc.dg/analyzer/torture/infoleak-net-ethtool-ioctl.c: New test. * gcc.dg/analyzer/torture/infoleak-vfio_iommu_type1.c: New test. Signed-off-by: David Malcolm --- gcc/Makefile.in | 3 +- gcc/analyzer/analyzer.opt | 4 + gcc/analyzer/checker-path.cc | 104 +++ gcc/analyzer/checker-path.h | 47 ++ gcc/analyzer/diagnostic-manager.cc | 75 ++- gcc/analyzer/diagnostic-manager.h | 3 +- gcc/analyzer/pending-diagnostic.cc | 30 + gcc/analyzer/pending-diagnostic.h | 24 + gcc/analyzer/region-model-impl-calls.cc | 11 + gcc/analyzer/region-model.cc | 457 ++++++++++++- gcc/analyzer/region-model.h | 19 +- gcc/analyzer/region.cc | 28 + gcc/analyzer/region.h | 4 + gcc/analyzer/store.h | 1 + gcc/analyzer/trust-boundaries.cc | 615 ++++++++++++++++++ gcc/doc/invoke.texi | 14 + .../gcc.dg/analyzer/copy-function-1.c | 98 +++ .../gcc.dg/analyzer/copy_from_user-1.c | 45 ++ gcc/testsuite/gcc.dg/analyzer/infoleak-1.c | 181 ++++++ gcc/testsuite/gcc.dg/analyzer/infoleak-2.c | 29 + gcc/testsuite/gcc.dg/analyzer/infoleak-3.c | 141 ++++ gcc/testsuite/gcc.dg/analyzer/infoleak-5.c | 35 + .../analyzer/infoleak-CVE-2011-1078-1.c | 134 ++++ .../analyzer/infoleak-CVE-2011-1078-2.c | 42 ++ .../analyzer/infoleak-CVE-2014-1446-1.c | 117 ++++ .../analyzer/infoleak-CVE-2017-18549-1.c | 101 +++ .../analyzer/infoleak-CVE-2017-18550-1.c | 171 +++++ .../gcc.dg/analyzer/infoleak-antipatterns-1.c | 162 +++++ .../gcc.dg/analyzer/infoleak-fixit-1.c | 22 + .../torture/infoleak-net-ethtool-ioctl.c | 78 +++ .../torture/infoleak-vfio_iommu_type1.c | 39 ++ 31 files changed, 2826 insertions(+), 8 deletions(-) create mode 100644 gcc/analyzer/trust-boundaries.cc create mode 100644 gcc/testsuite/gcc.dg/analyzer/copy-function-1.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/copy_from_user-1.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/infoleak-1.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/infoleak-2.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/infoleak-3.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/infoleak-5.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/infoleak-CVE-2011-1078-1.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/infoleak-CVE-2011-1078-2.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/infoleak-CVE-2014-1446-1.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/infoleak-CVE-2017-18549-1.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/infoleak-CVE-2017-18550-1.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/infoleak-antipatterns-1.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/infoleak-fixit-1.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/torture/infoleak-net-ethtool-ioctl.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/torture/infoleak-vfio_iommu_type1.c diff --git a/gcc/Makefile.in b/gcc/Makefile.in index 571e9c28e29..7e99dae0b1f 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -1273,7 +1273,8 @@ ANALYZER_OBJS = \ analyzer/store.o \ analyzer/supergraph.o \ analyzer/svalue.o \ - analyzer/trimmed-graph.o + analyzer/trimmed-graph.o \ + analyzer/trust-boundaries.o # Language-independent object files. # We put the *-match.o and insn-*.o files first so that a parallel make diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt index c85e30f8b11..fa8a3a94d79 100644 --- a/gcc/analyzer/analyzer.opt +++ b/gcc/analyzer/analyzer.opt @@ -66,6 +66,10 @@ Wanalyzer-exposure-through-output-file Common Var(warn_analyzer_exposure_through_output_file) Init(1) Warning Warn about code paths in which sensitive data is written to a file. +Wanalyzer-exposure-through-uninit-copy +Common Var(warn_analyzer_exposure_through_uninit_copy) Init(1) Warning +Warn about code paths in which sensitive data is copied across a security boundary. + Wanalyzer-file-leak Common Var(warn_analyzer_file_leak) Init(1) Warning Warn about code paths in which a stdio FILE is not closed. diff --git a/gcc/analyzer/checker-path.cc b/gcc/analyzer/checker-path.cc index e132f003470..d7b04322638 100644 --- a/gcc/analyzer/checker-path.cc +++ b/gcc/analyzer/checker-path.cc @@ -81,6 +81,8 @@ event_kind_to_string (enum event_kind ek) return "EK_CUSTOM"; case EK_STMT: return "EK_STMT"; + case EK_REGION_CREATION: + return "EK_REGION_CREATION"; case EK_FUNCTION_ENTRY: return "EK_FUNCTION_ENTRY"; case EK_STATE_CHANGE: @@ -199,6 +201,79 @@ statement_event::get_desc (bool) const return label_text::take (xstrdup (pp_formatted_text (&pp))); } +/* class region_creation_event : public checker_event. */ + +region_creation_event::region_creation_event (const region *reg, + tree capacity, + enum rce_kind kind, + location_t loc, + tree fndecl, + int depth) +: checker_event (EK_REGION_CREATION, loc, fndecl, depth), + m_reg (reg), + m_capacity (capacity), + m_rce_kind (kind) +{ + if (m_rce_kind == RCE_CAPACITY) + gcc_assert (capacity); +} + +/* Implementation of diagnostic_event::get_desc vfunc for + region_creation_event. + There are effectively 3 kinds of region_region_event, to + avoid combinatorial explosion by trying to convy the + information in a single message. */ + +label_text +region_creation_event::get_desc (bool can_colorize) const +{ + switch (m_rce_kind) + { + default: + gcc_unreachable (); + + case RCE_MEM_SPACE: + switch (m_reg->get_memory_space ()) + { + default: + return label_text::borrow ("region created here"); + case MEMSPACE_STACK: + return label_text::borrow ("region created on stack here"); + case MEMSPACE_HEAP: + return label_text::borrow ("region created on heap here"); + } + break; + + case RCE_CAPACITY: + gcc_assert (m_capacity); + if (TREE_CODE (m_capacity) == INTEGER_CST) + { + unsigned HOST_WIDE_INT hwi = tree_to_uhwi (m_capacity); + if (hwi == 1) + return make_label_text (can_colorize, + "capacity: %wu byte", hwi); + else + return make_label_text (can_colorize, + "capacity: %wu bytes", hwi); + } + else + return make_label_text (can_colorize, + "capacity: %qE bytes", m_capacity); + + case RCE_DEBUG: + { + pretty_printer pp; + pp_format_decoder (&pp) = default_tree_printer; + pp_string (&pp, "region creation: "); + m_reg->dump_to_pp (&pp, true); + if (m_capacity) + pp_printf (&pp, " capacity: %qE", m_capacity); + return label_text::take (xstrdup (pp_formatted_text (&pp))); + } + break; + } +} + /* class function_entry_event : public checker_event. */ /* Implementation of diagnostic_event::get_desc vfunc for @@ -991,6 +1066,35 @@ checker_path::debug () const } } +/* Add region_creation_event instances to this path for REG, + describing whether REG is on the stack or heap and what + its capacity is (if known). + If DEBUG is true, also create an RCE_DEBUG event. */ + +void +checker_path::add_region_creation_events (const region *reg, + const region_model *model, + location_t loc, + tree fndecl, int depth, + bool debug) +{ + tree capacity = NULL_TREE; + if (model) + if (const svalue *capacity_sval = model->get_capacity (reg)) + capacity = model->get_representative_tree (capacity_sval); + + add_event (new region_creation_event (reg, capacity, RCE_MEM_SPACE, + loc, fndecl, depth)); + + if (capacity) + add_event (new region_creation_event (reg, capacity, RCE_CAPACITY, + loc, fndecl, depth)); + + if (debug) + add_event (new region_creation_event (reg, capacity, RCE_DEBUG, + loc, fndecl, depth)); +} + /* Add a warning_event to the end of this path. */ void diff --git a/gcc/analyzer/checker-path.h b/gcc/analyzer/checker-path.h index 27634c20864..63b11f13037 100644 --- a/gcc/analyzer/checker-path.h +++ b/gcc/analyzer/checker-path.h @@ -31,6 +31,7 @@ enum event_kind EK_DEBUG, EK_CUSTOM, EK_STMT, + EK_REGION_CREATION, EK_FUNCTION_ENTRY, EK_STATE_CHANGE, EK_START_CFG_EDGE, @@ -58,6 +59,7 @@ extern const char *event_kind_to_string (enum event_kind ek); custom_event (EK_CUSTOM) precanned_custom_event statement_event (EK_STMT) + region_creation_event (EK_REGION_CREATION) function_entry_event (EK_FUNCTION_ENTRY) state_change_event (EK_STATE_CHANGE) superedge_event @@ -194,6 +196,45 @@ public: const program_state m_dst_state; }; +/* There are too many combinations to express region creation in one message, + so we emit multiple region_creation_event instances when each pertinent + region is created. + + This enum distinguishes between the different messages. */ + +enum rce_kind +{ + /* Generate a message based on the memory space of the region + e.g. "region created on stack here". */ + RCE_MEM_SPACE, + + /* Generate a message based on the capacity of the region + e.g. "capacity: 100 bytes". */ + RCE_CAPACITY, + + /* Generate a debug message. */ + RCE_DEBUG +}; + +/* A concrete event subclass describing the creation of a region that + is significant for a diagnostic. */ + +class region_creation_event : public checker_event +{ +public: + region_creation_event (const region *reg, + tree capacity, + enum rce_kind kind, + location_t loc, tree fndecl, int depth); + + label_text get_desc (bool) const FINAL OVERRIDE; + +private: + const region *m_reg; + tree m_capacity; + enum rce_kind m_rce_kind; +}; + /* An event subclass describing the entry to a function. */ class function_entry_event : public checker_event @@ -561,6 +602,12 @@ public: m_events[idx] = new_event; } + void add_region_creation_events (const region *reg, + const region_model *model, + location_t loc, + tree fndecl, int depth, + bool debug); + void add_final_event (const state_machine *sm, const exploded_node *enode, const gimple *stmt, tree var, state_machine::state_t state); diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc index 7ffe0004356..fdefa96d2f1 100644 --- a/gcc/analyzer/diagnostic-manager.cc +++ b/gcc/analyzer/diagnostic-manager.cc @@ -1219,7 +1219,7 @@ diagnostic_manager::emit_saved_diagnostic (const exploded_graph &eg, trailing eedge stashed, add any events for it. This is for use in handling longjmp, to show where a longjmp is rewinding to. */ if (sd.m_trailing_eedge) - add_events_for_eedge (pb, *sd.m_trailing_eedge, &emission_path); + add_events_for_eedge (pb, *sd.m_trailing_eedge, &emission_path, NULL); emission_path.prepare_for_emission (sd.m_d); @@ -1266,10 +1266,13 @@ diagnostic_manager::build_emission_path (const path_builder &pb, checker_path *emission_path) const { LOG_SCOPE (get_logger ()); + + interesting_t interest; + pb.get_pending_diagnostic ()->mark_interesting_stuff (&interest); for (unsigned i = 0; i < epath.m_edges.length (); i++) { const exploded_edge *eedge = epath.m_edges[i]; - add_events_for_eedge (pb, *eedge, emission_path); + add_events_for_eedge (pb, *eedge, emission_path, &interest); } } @@ -1577,10 +1580,12 @@ struct null_assignment_sm_context : public sm_context void diagnostic_manager::add_events_for_eedge (const path_builder &pb, const exploded_edge &eedge, - checker_path *emission_path) const + checker_path *emission_path, + interesting_t *interest) const { const exploded_node *src_node = eedge.m_src; const program_point &src_point = src_node->get_point (); + const int src_stack_depth = src_point.get_stack_depth (); const exploded_node *dst_node = eedge.m_dest; const program_point &dst_point = dst_node->get_point (); const int dst_stack_depth = dst_point.get_stack_depth (); @@ -1642,6 +1647,29 @@ diagnostic_manager::add_events_for_eedge (const path_builder &pb, (dst_point.get_supernode ()->get_start_location (), dst_point.get_fndecl (), dst_stack_depth)); + /* Create region_creation_events for on-stack regions within + this frame. */ + if (interest) + { + unsigned i; + const region *reg; + FOR_EACH_VEC_ELT (interest->m_region_creation, i, reg) + if (const frame_region *frame = reg->maybe_get_frame_region ()) + if (frame->get_fndecl () == dst_point.get_fndecl ()) + { + const region *base_reg = reg->get_base_region (); + if (tree decl = base_reg->maybe_get_decl ()) + if (DECL_SOURCE_LOCATION (decl) != UNKNOWN_LOCATION) + { + emission_path->add_region_creation_events + (reg, dst_state.m_region_model, + DECL_SOURCE_LOCATION (decl), + dst_point.get_fndecl (), + dst_stack_depth, + m_verbosity > 3); + } + } + } } break; case PK_BEFORE_STMT: @@ -1697,6 +1725,43 @@ diagnostic_manager::add_events_for_eedge (const path_builder &pb, == dst_node->m_succs[0]->m_dest->get_point ()))) break; } + + /* Look for changes in dynamic extents, which will identify + the creation of heap-based regions and alloca regions. */ + if (interest) + { + const region_model *src_model = src_state.m_region_model; + const region_model *dst_model = dst_state.m_region_model; + if (src_model->get_dynamic_extents () + != dst_model->get_dynamic_extents ()) + { + unsigned i; + const region *reg; + FOR_EACH_VEC_ELT (interest->m_region_creation, i, reg) + { + const region *base_reg = reg->get_base_region (); + const svalue *old_extents + = src_model->get_dynamic_extents (base_reg); + const svalue *new_extents + = dst_model->get_dynamic_extents (base_reg); + if (old_extents == NULL && new_extents != NULL) + switch (base_reg->get_kind ()) + { + default: + break; + case RK_HEAP_ALLOCATED: + case RK_ALLOCA: + emission_path->add_region_creation_events + (reg, dst_model, + src_point.get_location (), + src_point.get_fndecl (), + src_stack_depth, + m_verbosity > 3); + break; + } + } + } + } } } break; @@ -2001,6 +2066,10 @@ diagnostic_manager::prune_for_sm_diagnostic (checker_path *path, } break; + case EK_REGION_CREATION: + /* Don't filter these. */ + break; + case EK_FUNCTION_ENTRY: if (m_verbosity < 1) { diff --git a/gcc/analyzer/diagnostic-manager.h b/gcc/analyzer/diagnostic-manager.h index ad2eb4dfe65..b4836aaf532 100644 --- a/gcc/analyzer/diagnostic-manager.h +++ b/gcc/analyzer/diagnostic-manager.h @@ -141,7 +141,8 @@ private: void add_events_for_eedge (const path_builder &pb, const exploded_edge &eedge, - checker_path *emission_path) const; + checker_path *emission_path, + interesting_t *interest) const; bool significant_edge_p (const path_builder &pb, const exploded_edge &eedge) const; diff --git a/gcc/analyzer/pending-diagnostic.cc b/gcc/analyzer/pending-diagnostic.cc index adff25130fd..a0c1db4ebcc 100644 --- a/gcc/analyzer/pending-diagnostic.cc +++ b/gcc/analyzer/pending-diagnostic.cc @@ -33,11 +33,41 @@ along with GCC; see the file COPYING3. If not see #include "diagnostic-event-id.h" #include "analyzer/sm.h" #include "analyzer/pending-diagnostic.h" +#include "selftest.h" +#include "tristate.h" +#include "analyzer/call-string.h" +#include "analyzer/program-point.h" +#include "analyzer/store.h" +#include "analyzer/region-model.h" #if ENABLE_ANALYZER namespace ana { +/* struct interesting_t. */ + +void +interesting_t::add_region_creation (const region *reg) +{ + gcc_assert (reg); + m_region_creation.safe_push (reg); +} + +void +interesting_t::dump_to_pp (pretty_printer *pp, bool simple) const +{ + pp_string (pp, "{ region creation: ["); + unsigned i; + const region *reg; + FOR_EACH_VEC_ELT (m_region_creation, i, reg) + { + if (i > 0) + pp_string (pp, ", "); + reg->dump_to_pp (pp, simple); + } + pp_string (pp, "]}"); +} + /* Generate a label_text by printing FMT. Use a clone of the global_dc for formatting callbacks. diff --git a/gcc/analyzer/pending-diagnostic.h b/gcc/analyzer/pending-diagnostic.h index 48e2b3ec171..3d57afabae8 100644 --- a/gcc/analyzer/pending-diagnostic.h +++ b/gcc/analyzer/pending-diagnostic.h @@ -23,6 +23,22 @@ along with GCC; see the file COPYING3. If not see namespace ana { +/* A bundle of information about things that are of interest to a + pending_diagnostic. + + For now, merely the set of regions that are pertinent to the + diagnostic, so that we can notify the user about when they + were created. */ + +struct interesting_t +{ + void add_region_creation (const region *reg); + + void dump_to_pp (pretty_printer *pp, bool simple) const; + + auto_vec m_region_creation; +}; + /* Various bundles of information used for generating more precise messages for events within a diagnostic_path, for passing to the various "describe_*" vfuncs of pending_diagnostic. See those @@ -282,6 +298,14 @@ class pending_diagnostic { return false; } + + /* Vfunc for registering additional information of interest to this + diagnostic. */ + + virtual void mark_interesting_stuff (interesting_t *) + { + /* Default no-op implementation. */ + } }; /* A template to make it easier to make subclasses of pending_diagnostic. diff --git a/gcc/analyzer/region-model-impl-calls.cc b/gcc/analyzer/region-model-impl-calls.cc index 90d4cf9c2db..f4b19e8cd7b 100644 --- a/gcc/analyzer/region-model-impl-calls.cc +++ b/gcc/analyzer/region-model-impl-calls.cc @@ -80,6 +80,17 @@ call_details::call_details (const gcall *call, region_model *model, } } +/* Get any logger associated with this object. */ + +logger * +call_details::get_logger () const +{ + if (m_ctxt) + return m_ctxt->get_logger (); + else + return NULL; +} + /* Get any uncertainty_t associated with the region_model_context. */ uncertainty_t * diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index 416a5ac7249..b4d60812271 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -67,6 +67,7 @@ along with GCC; see the file COPYING3. If not see #include "stor-layout.h" #include "attribs.h" #include "tree-object-size.h" +#include "analyzer/call-info.h" #if ENABLE_ANALYZER @@ -1041,6 +1042,445 @@ region_model::on_stmt_pre (const gimple *stmt, } } +/* An enum for capturing the presence of one of + __attribute (("returns_zero_on_failure")) or + __attribute (("returns_zero_on_success")). */ + +enum return_meaning +{ + RETURN_MEANING_UNKNOWN, // or no return value + RETURN_MEANING_ZERO_ON_FAILURE, + RETURN_MEANING_ZERO_ON_SUCCESS +}; + +/* Determine if FNDECL has been marked with one of + __attribute (("returns_zero_on_failure")) or + __attribute (("returns_zero_on_success")). */ + +static enum return_meaning +get_return_meaning (tree fndecl) +{ + tree attrs = TYPE_ATTRIBUTES (TREE_TYPE (fndecl)); + if (lookup_attribute ("returns_zero_on_failure", attrs)) + return RETURN_MEANING_ZERO_ON_FAILURE; + if (lookup_attribute ("returns_zero_on_success", attrs)) + return RETURN_MEANING_ZERO_ON_SUCCESS; + return RETURN_MEANING_UNKNOWN; +} + +/* Update this model for an outcome of a call that returns zero. */ + +void +region_model::update_for_zero_return (const call_details &cd) +{ + if (!cd.get_lhs_type ()) + return; + const svalue *zero + = m_mgr->get_or_create_int_cst (cd.get_lhs_type (), 0); + /* Need to make this unmergeable to prevent the state-merger code + from merging the success and failure outcomes. */ + set_value (cd.get_lhs_region (), + m_mgr->get_or_create_unmergeable (zero), + cd.get_ctxt ()); +} + +/* Update this model for an outcome of a call that returns non-zero. */ + +void +region_model::update_for_nonzero_return (const call_details &cd) +{ + if (!cd.get_lhs_type ()) + return; + const svalue *zero + = m_mgr->get_or_create_int_cst (cd.get_lhs_type (), 0); + const svalue *result + = get_store_value (cd.get_lhs_region (), cd.get_ctxt ()); + add_constraint (result, NE_EXPR, zero, cd.get_ctxt ()); +} + +/* Subclass of call_info for a that call that succeeds or fails, where + the return value is zero or nonzero to signify success/failure + adding a "when `FNDECL' succeeds/fails returning zero/nonzero" message. + (giving four combinations: success vs failure and zero vs nonzero). + + This is still abstract: the custom_edge_info::update_model vfunc + must be implemented. */ + +class maybe_returns_zero_call_info : public call_info +{ +public: + maybe_returns_zero_call_info (const call_details &cd, + enum return_meaning return_meaning, + bool success) + : call_info (cd), + m_return_meaning (return_meaning), + m_success (success) + {} + + label_text get_desc (bool can_colorize) const + { + switch (m_return_meaning) + { + default: + case RETURN_MEANING_UNKNOWN: + gcc_unreachable (); + break; + case RETURN_MEANING_ZERO_ON_FAILURE: + if (m_success) + return make_label_text (can_colorize, + "when %qE succeeds, returning non-zero", + get_fndecl ()); + else + return make_label_text (can_colorize, + "when %qE fails, returning zero", + get_fndecl ()); + case RETURN_MEANING_ZERO_ON_SUCCESS: + if (m_success) + return make_label_text (can_colorize, + "when %qE succeeds, returning zero", + get_fndecl ()); + else + return make_label_text (can_colorize, + "when %qE fails, returning non-zero", + get_fndecl ()); + } + } + +protected: + void update_model_for_return_value (region_model *model, + region_model_context *ctxt) const + { + const call_details cd (get_call_details (model, ctxt)); + + /* Update return value. */ + switch (m_return_meaning) + { + default: + case RETURN_MEANING_UNKNOWN: + gcc_unreachable (); + break; + case RETURN_MEANING_ZERO_ON_FAILURE: + if (m_success) + model->update_for_nonzero_return (cd); + else + model->update_for_zero_return (cd); + break; + case RETURN_MEANING_ZERO_ON_SUCCESS: + if (m_success) + model->update_for_zero_return (cd); + else + model->update_for_nonzero_return (cd); + break; + } + } + + enum return_meaning m_return_meaning; + bool m_success; +}; + +/* Concrete custom_edge_info: a copy that succeeds/fails, + with an appropriate return value. */ + +class copy_success : public maybe_returns_zero_call_info +{ +public: + copy_success (const call_details &cd, + enum return_meaning return_meaning, + const region *sized_dest_reg, + const svalue *copied_sval, + const region *sized_src_reg) + : maybe_returns_zero_call_info (cd, return_meaning, true), + m_sized_dest_reg (sized_dest_reg), + m_copied_sval (copied_sval), + m_sized_src_reg (sized_src_reg) + {} + + bool update_model (region_model *model, + const exploded_edge *, + region_model_context *ctxt) const FINAL OVERRIDE + { + update_model_for_return_value (model, ctxt); + model->set_value (m_sized_dest_reg, m_copied_sval, ctxt, m_sized_src_reg); + return true; + } + + const region *m_sized_dest_reg; + const svalue *m_copied_sval; + const region *m_sized_src_reg; +}; + +/* Concrete custom_edge_info: a copy that fails, + with an appropriate return value. */ + +class copy_failure : public maybe_returns_zero_call_info +{ +public: + copy_failure (const call_details &cd, + enum return_meaning return_meaning) + : maybe_returns_zero_call_info (cd, return_meaning, false) + {} + + bool update_model (region_model *model, + const exploded_edge *, + region_model_context *ctxt) const FINAL OVERRIDE + { + update_model_for_return_value (model, ctxt); + /* Leave the destination region untouched. */ + return true; + } + +private: + enum return_meaning m_return_meaning; +}; + +/* The Linux kernel commonly uses + min_t([unsigned] long, VAR, sizeof(T)); + to set an upper bound on the size of a copy_to_user. + Attempt to simplify such sizes by trying to get the upper bound as a + constant. + Return the simplified svalue if possible, or NULL otherwise. */ + +static const svalue * +maybe_simplify_upper_bound (const svalue *num_bytes_sval, + region_model_manager *mgr) +{ + tree type = num_bytes_sval->get_type (); + while (const svalue *raw = num_bytes_sval->maybe_undo_cast ()) + num_bytes_sval = raw; + if (const binop_svalue *binop_sval = num_bytes_sval->dyn_cast_binop_svalue ()) + if (binop_sval->get_op () == MIN_EXPR) + if (binop_sval->get_arg1 ()->get_kind () == SK_CONSTANT) + { + return mgr->get_or_create_cast (type, binop_sval->get_arg1 ()); + /* TODO: we might want to also capture the constraint + when recording the diagnostic, or note that we're using + the upper bound. */ + } + return NULL; +} + +/* Attempt to get an upper bound for the size of a copy when simulating a + copy function. + + NUM_BYTES_SVAL is the symbolic value for the size of the copy. + Use it if it's constant, otherwise try to simplify it. Failing + that, use the size of SRC_REG if constant. + + Return a symbolic value for an upper limit on the number of bytes + copied, or NULL if no such value could be determined. */ + +const svalue * +region_model::maybe_get_copy_bounds (const region *src_reg, + const svalue *num_bytes_sval) +{ + if (num_bytes_sval->maybe_get_constant ()) + return num_bytes_sval; + + if (const svalue *simplified + = maybe_simplify_upper_bound (num_bytes_sval, m_mgr)) + num_bytes_sval = simplified; + + if (num_bytes_sval->maybe_get_constant ()) + return num_bytes_sval; + + /* For now, try just guessing the size as the capacity of the + base region of the src. + This is a hack; we might get too large a value. */ + const region *src_base_reg = src_reg->get_base_region (); + num_bytes_sval = get_capacity (src_base_reg); + + if (num_bytes_sval->maybe_get_constant ()) + return num_bytes_sval; + + /* Non-constant: give up. */ + return NULL; +} + +/* Support for "copy functions". + + Heuristic: if a function has just an access((read_only)), + access((write_only)) and a size param (in any order), assume it copies + between buffers, and has no side effects. */ + +/* Struct for capturing the order of the params of a copy function. */ + +struct copy_fn_details +{ + int m_src_arg_idx; + int m_dst_arg_idx; + int m_sz_arg_idx; +}; + +/* Return true if FNDECL is a copy function and populate *OUT. + Return false otherwise. */ + +static bool +is_copy_function (tree fndecl, copy_fn_details *out) +{ + tree fntype = TREE_TYPE (fndecl); + /* Must have exactly 3 args. */ + if (type_num_arguments (fntype) != 3) + return false; + /* Reject variadic functions. */ + if (type_argument_type (fntype, 4) == NULL_TREE) + return false; + + rdwr_map rwm; + init_attr_rdwr_indices (&rwm, TYPE_ATTRIBUTES (fntype)); + + if (rwm.elements () != 3) + return false; + + int src_ptr_arg_idx = -1; + int src_sz_arg_idx = -1; + int dst_ptr_arg_idx = -1; + int dst_sz_arg_idx = -1; + + for (unsigned arg_idx = 0; arg_idx < 3; arg_idx++) + { + const attr_access *access = rwm.get (arg_idx); + if (!access) + return false; + switch (access->mode) + { + default: + gcc_unreachable (); + case access_none: + return false; + + case access_read_only: + if (src_ptr_arg_idx != -1 + && src_ptr_arg_idx != (int)access->ptrarg) + /* More than one read_only. */ + return false; + src_ptr_arg_idx = access->ptrarg; + src_sz_arg_idx = access->sizarg; + break; + + case access_write_only: + if (dst_ptr_arg_idx != -1 + && dst_ptr_arg_idx != (int)access->ptrarg) + /* More than one write_only. */ + return false; + dst_ptr_arg_idx = access->ptrarg; + dst_sz_arg_idx = access->sizarg; + break; + + case access_read_write: + case access_deferred: + return false; + } + } + + if (src_ptr_arg_idx == -1 + || src_sz_arg_idx == -1 + || dst_ptr_arg_idx == -1 + || dst_sz_arg_idx == -1 + || src_sz_arg_idx != dst_sz_arg_idx + || src_ptr_arg_idx == src_sz_arg_idx) + return false; + + /* The size param must have integer type; this is checked by the attribute + handler. + + type_argument_type takes a 1-based argno, rather than a 0-based arg + idx. */ + gcc_assert (INTEGRAL_TYPE_P (type_argument_type (fntype, + src_sz_arg_idx + 1))); + + /* If we get here we have a pair: + access (read_only, A), access (write_only, A). */ + out->m_src_arg_idx = src_ptr_arg_idx; + out->m_dst_arg_idx = dst_ptr_arg_idx; + out->m_sz_arg_idx = src_sz_arg_idx; + return true; +} + +/* Update this model assuming that CD is a call to CALLEE_FNDECL, a + copy function described by CFD. */ + +void +region_model::handle_copy_function (tree callee_fndecl, + const call_details &cd, + const copy_fn_details &cfd) +{ + LOG_SCOPE (cd.get_logger ()); + + gcc_assert (callee_fndecl); + enum return_meaning return_meaning = get_return_meaning (callee_fndecl); + + const svalue *dest_sval = cd.get_arg_svalue (cfd.m_dst_arg_idx); + const svalue *src_sval = cd.get_arg_svalue (cfd.m_src_arg_idx); + const svalue *num_bytes_sval = cd.get_arg_svalue (cfd.m_sz_arg_idx); + + const region *dest_reg = deref_rvalue (dest_sval, + cd.get_arg_tree (cfd.m_dst_arg_idx), + cd.get_ctxt ()); + const region *src_reg = deref_rvalue (src_sval, + cd.get_arg_tree (cfd.m_src_arg_idx), + cd.get_ctxt ()); + + if (const svalue * bounded_sval = maybe_get_copy_bounds (src_reg, + num_bytes_sval)) + num_bytes_sval = bounded_sval; + + if (tree cst = num_bytes_sval->maybe_get_constant ()) + if (zerop (cst)) + /* No-op. */ + return; + + const region *sized_src_reg = m_mgr->get_sized_region (src_reg, + NULL_TREE, + num_bytes_sval); + + const svalue *copied_sval = get_store_value (sized_src_reg, cd.get_ctxt ()); + + const region *sized_dest_reg = m_mgr->get_sized_region (dest_reg, + NULL_TREE, + num_bytes_sval); + + /* Heuristic for handling copies that can fail. + + In the Linux kernel, the functions copy_to_user and copy_from_user copy + an arbitrary amount of data to/from userspace. They return the amount + of uncopied data (i.e. zero means success, nonzero means failure). + + Support this kind of copy function by bifurcating the state into + success (all of N was copied) and failure (none was copied). We + don't bother with the "only a fraction was copied" case. + + This heuristic should help find problems in error-handling without + overcomplicated the analysis. */ + if (cd.get_ctxt ()) + { + switch (return_meaning) + { + default: + gcc_unreachable (); + case RETURN_MEANING_UNKNOWN: + /* Don't bifurcate state; assume a full copy. */ + set_value (sized_dest_reg, copied_sval, cd.get_ctxt (), + sized_src_reg); + break; + + case RETURN_MEANING_ZERO_ON_FAILURE: + case RETURN_MEANING_ZERO_ON_SUCCESS: + { + /* Bifurcate state, creating a "failure" out-edge. */ + cd.get_ctxt ()->bifurcate (new copy_failure (cd, return_meaning)); + + /* The "unbifurcated" state is the "success" case. */ + copy_success success (cd, return_meaning, + sized_dest_reg, + copied_sval, + sized_src_reg); + success.update_model (this, NULL, cd.get_ctxt ()); + } + break; + } + } +} + /* Update this model for the CALL stmt, using CTXT to report any diagnostics - the first half. @@ -1092,6 +1532,8 @@ region_model::on_call_pre (const gcall *call, region_model_context *ctxt, if (tree callee_fndecl = get_fndecl_for_call (call, ctxt)) { + copy_fn_details cfd; + /* The various impl_call_* member functions are implemented in region-model-impl-calls.cc. Having them split out into separate functions makes it easier @@ -1247,6 +1689,11 @@ region_model::on_call_pre (const gcall *call, region_model_context *ctxt, { /* Handle in "on_call_post". */ } + else if (is_copy_function (callee_fndecl, &cfd)) + { + handle_copy_function (callee_fndecl, cd, cfd); + return false; + } else if (!fndecl_has_gimple_body_p (callee_fndecl) && !DECL_PURE_P (callee_fndecl) && !fndecl_built_in_p (callee_fndecl)) @@ -2336,17 +2783,23 @@ region_model::check_region_for_read (const region *src_reg, /* Set the value of the region given by LHS_REG to the value given by RHS_SVAL. - Use CTXT to report any warnings associated with writing to LHS_REG. */ + Use CTXT to report any warnings associated with writing to LHS_REG. + SRC_REG can be NULL, if non-NULL it's a hint about where RHS_SVAL + came from, for precision-of-wording in diagnostics. */ void region_model::set_value (const region *lhs_reg, const svalue *rhs_sval, - region_model_context *ctxt) + region_model_context *ctxt, + const region *src_reg) { gcc_assert (lhs_reg); gcc_assert (rhs_sval); check_region_for_write (lhs_reg, ctxt); + if (ctxt && lhs_reg->untrusted_p ()) + maybe_complain_about_infoleak (lhs_reg, rhs_sval, src_reg, ctxt); + m_store.set_value (m_mgr->get_store_manager(), lhs_reg, rhs_sval, ctxt ? ctxt->get_uncertainty () : NULL); } diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h index 13e8109aa51..322f8d5807b 100644 --- a/gcc/analyzer/region-model.h +++ b/gcc/analyzer/region-model.h @@ -479,6 +479,8 @@ public: region_model_context *ctxt); region_model_context *get_ctxt () const { return m_ctxt; } + logger *get_logger () const; + uncertainty_t *get_uncertainty () const; tree get_lhs_type () const { return m_lhs_type; } const region *get_lhs_region () const { return m_lhs_region; } @@ -509,6 +511,8 @@ private: const region *m_lhs_region; }; +struct copy_fn_details; + /* A region_model encapsulates a representation of the state of memory, with a tree of regions, along with their associated values. The representation is graph-like because values can be pointers to @@ -591,6 +595,13 @@ class region_model void impl_call_operator_new (const call_details &cd); void impl_call_operator_delete (const call_details &cd); void impl_deallocation_call (const call_details &cd); + void handle_copy_function (tree callee_fndecl, + const call_details &cd, + const copy_fn_details &cfd); + const svalue *maybe_get_copy_bounds (const region *src_reg, + const svalue *num_bytes_sval); + void update_for_zero_return (const call_details &cd); + void update_for_nonzero_return (const call_details &cd); void handle_unrecognized_call (const gcall *call, region_model_context *ctxt); @@ -648,7 +659,8 @@ class region_model region_model_context *ctxt) const; void set_value (const region *lhs_reg, const svalue *rhs_sval, - region_model_context *ctxt); + region_model_context *ctxt, + const region *src_reg = NULL); void set_value (tree lhs, tree rhs, region_model_context *ctxt); void clobber_region (const region *reg); void purge_region (const region *reg); @@ -813,6 +825,11 @@ class region_model void check_region_for_read (const region *src_reg, region_model_context *ctxt) const; + void maybe_complain_about_infoleak (const region *dst_reg, + const svalue *copied_sval, + const region *src_reg, + region_model_context *ctxt); + /* Storing this here to avoid passing it around everywhere. */ region_model_manager *const m_mgr; diff --git a/gcc/analyzer/region.cc b/gcc/analyzer/region.cc index fa187fde331..bb4f53b8802 100644 --- a/gcc/analyzer/region.cc +++ b/gcc/analyzer/region.cc @@ -204,6 +204,34 @@ region::get_memory_space () const return MEMSPACE_UNKNOWN; } +/* Get the address space of this region. */ + +addr_space_t +region::get_addr_space () const +{ + const region *iter = this; + while (iter) + { + if (iter->m_type) + return TYPE_ADDR_SPACE (iter->m_type); + switch (iter->get_kind ()) + { + case RK_FIELD: + case RK_ELEMENT: + case RK_OFFSET: + case RK_SIZED: + iter = iter->get_parent_region (); + continue; + case RK_CAST: + iter = iter->dyn_cast_cast_region ()->get_original_region (); + continue; + default: + return ADDR_SPACE_GENERIC; + } + } + return ADDR_SPACE_GENERIC; +} + /* Subroutine for use by region_model_manager::get_or_create_initial_value. Return true if this region has an initial_svalue. Return false if attempting to use INIT_VAL(this_region) should give diff --git a/gcc/analyzer/region.h b/gcc/analyzer/region.h index a17e73c30c9..58e28b998aa 100644 --- a/gcc/analyzer/region.h +++ b/gcc/analyzer/region.h @@ -136,6 +136,7 @@ public: bool descendent_of_p (const region *elder) const; const frame_region *maybe_get_frame_region () const; enum memory_space get_memory_space () const; + addr_space_t get_addr_space () const; bool can_have_initial_svalue_p () const; tree maybe_get_decl () const; @@ -189,6 +190,8 @@ public: const complexity &get_complexity () const { return m_complexity; } + bool untrusted_p () const; + protected: region (complexity c, unsigned id, const region *parent, tree type); @@ -296,6 +299,7 @@ public: /* Accessors. */ const frame_region *get_calling_frame () const { return m_calling_frame; } function *get_function () const { return m_fun; } + tree get_fndecl () const { return get_function ()->decl; } int get_index () const { return m_index; } int get_stack_depth () const { return m_index + 1; } diff --git a/gcc/analyzer/store.h b/gcc/analyzer/store.h index da82bd1bdec..fa824f0eee4 100644 --- a/gcc/analyzer/store.h +++ b/gcc/analyzer/store.h @@ -558,6 +558,7 @@ public: hashval_t hash () const; bool symbolic_p () const; + const region *get_base_region () const { return m_base_region; } void dump_to_pp (pretty_printer *pp, bool simple, bool multiline) const; void dump (bool simple) const; diff --git a/gcc/analyzer/trust-boundaries.cc b/gcc/analyzer/trust-boundaries.cc new file mode 100644 index 00000000000..0425531cd42 --- /dev/null +++ b/gcc/analyzer/trust-boundaries.cc @@ -0,0 +1,615 @@ +/* Handling of trust boundaries + Copyright (C) 2019-2021 Free Software Foundation, Inc. + Contributed by David Malcolm . + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify it +under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, but +WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +General Public License for more details. + +You should have received a copy of the GNU General Public License +along with GCC; see the file COPYING3. If not see +. */ + +#include "config.h" +#define INCLUDE_UNIQUE_PTR +#include "system.h" +#include "coretypes.h" +#include "tree.h" +#include "diagnostic-core.h" +#include "tree-pretty-print.h" +#include "diagnostic-metadata.h" +#include "tristate.h" +#include "selftest.h" +#include "function.h" +#include "json.h" +#include "analyzer/analyzer.h" +#include "analyzer/analyzer-logging.h" +#include "digraph.h" +#include "analyzer/call-string.h" +#include "analyzer/program-point.h" +#include "analyzer/store.h" +#include "analyzer/region-model.h" +#include "diagnostic-event-id.h" +#include "analyzer/sm.h" +#include "analyzer/pending-diagnostic.h" +#include "gcc-rich-location.h" + +#if ENABLE_ANALYZER + +namespace ana { + +/* Information of the layout of a RECORD_TYPE, capturing it as a vector + of items, where each item is either a field or padding. */ + +class record_layout +{ +public: + /* An item within a record; either a field, or padding after a field. */ + struct item + { + public: + item (const bit_range &br, + tree field, + bool is_padding) + : m_bit_range (br), + m_field (field), + m_is_padding (is_padding) + { + } + + bit_offset_t get_start_bit_offset () const + { + return m_bit_range.get_start_bit_offset (); + } + bit_offset_t get_next_bit_offset () const + { + return m_bit_range.get_next_bit_offset (); + } + + bool contains_p (bit_offset_t offset) const + { + return m_bit_range.contains_p (offset); + } + + void dump_to_pp (pretty_printer *pp) const + { + if (m_is_padding) + pp_printf (pp, "padding after %qD", m_field); + else + pp_printf (pp, "%qD", m_field); + pp_string (pp, ", "); + m_bit_range.dump_to_pp (pp); + } + + bit_range m_bit_range; + tree m_field; + bool m_is_padding; + }; + + record_layout (tree record_type) + : m_record_type (record_type) + { + gcc_assert (TREE_CODE (record_type) == RECORD_TYPE); + + for (tree iter = TYPE_FIELDS (record_type); iter != NULL_TREE; + iter = DECL_CHAIN (iter)) + { + if (TREE_CODE (iter) == FIELD_DECL) + { + int iter_field_offset = int_bit_position (iter); + bit_size_t size_in_bits; + if (!int_size_in_bits (TREE_TYPE (iter), &size_in_bits)) + size_in_bits = 0; + + maybe_pad_to (iter_field_offset); + + /* Add field. */ + m_items.safe_push (item (bit_range (iter_field_offset, + size_in_bits), + iter, false)); + } + } + + /* Add any trailing padding. */ + bit_size_t size_in_bits; + if (int_size_in_bits (record_type, &size_in_bits)) + maybe_pad_to (size_in_bits); + } + + void dump_to_pp (pretty_printer *pp) const + { + unsigned i; + item *it; + FOR_EACH_VEC_ELT (m_items, i, it) + { + it->dump_to_pp (pp); + pp_newline (pp); + } + } + + DEBUG_FUNCTION void dump () const + { + pretty_printer pp; + pp_format_decoder (&pp) = default_tree_printer; + pp.buffer->stream = stderr; + dump_to_pp (&pp); + pp_flush (&pp); + } + + const record_layout::item *get_item_at (bit_offset_t offset) const + { + unsigned i; + item *it; + FOR_EACH_VEC_ELT (m_items, i, it) + if (it->contains_p (offset)) + return it; + return NULL; + } + +private: + /* Subroutine of ctor. Add padding item to NEXT_OFFSET if necessary. */ + + void maybe_pad_to (bit_offset_t next_offset) + { + if (m_items.length () > 0) + { + const item &last_item = m_items[m_items.length () - 1]; + bit_offset_t offset_after_last_item + = last_item.get_next_bit_offset (); + if (next_offset > offset_after_last_item) + { + bit_size_t padding_size + = next_offset - offset_after_last_item; + m_items.safe_push (item (bit_range (offset_after_last_item, + padding_size), + last_item.m_field, true)); + } + } + } + + tree m_record_type; + auto_vec m_items; +}; + +/* A subclass of pending_diagnostic for complaining about uninitialized data + being copied across a trust boundary to an untrusted output + (e.g. copy_to_user infoleaks in the Linux kernel). */ + +class exposure_through_uninit_copy + : public pending_diagnostic_subclass +{ +public: + exposure_through_uninit_copy (const region *src_region, + const region *dest_region, + const svalue *copied_sval, + region_model_manager *mgr) + : m_src_region (src_region), + m_dest_region (dest_region), + m_copied_sval (copied_sval), + m_mgr (mgr) + { + gcc_assert (m_copied_sval->get_kind () == SK_POISONED + || m_copied_sval->get_kind () == SK_COMPOUND); + } + + const char *get_kind () const FINAL OVERRIDE + { + return "exposure_through_uninit_copy"; + } + + bool operator== (const exposure_through_uninit_copy &other) const + { + return (m_src_region == other.m_src_region + && m_dest_region == other.m_dest_region + && m_copied_sval == other.m_copied_sval); + } + + bool emit (rich_location *rich_loc) FINAL OVERRIDE + { + diagnostic_metadata m; + /* CWE-200: Exposure of Sensitive Information to an Unauthorized Actor. */ + m.add_cwe (200); + enum memory_space mem_space = get_src_memory_space (); + bool warned; + switch (mem_space) + { + default: + warned = warning_meta + (rich_loc, m, + OPT_Wanalyzer_exposure_through_uninit_copy, + "potential exposure of sensitive information" + " by copying uninitialized data across trust boundary"); + break; + case MEMSPACE_STACK: + warned = warning_meta + (rich_loc, m, + OPT_Wanalyzer_exposure_through_uninit_copy, + "potential exposure of sensitive information" + " by copying uninitialized data from stack across trust boundary"); + break; + case MEMSPACE_HEAP: + warned = warning_meta + (rich_loc, m, + OPT_Wanalyzer_exposure_through_uninit_copy, + "potential exposure of sensitive information" + " by copying uninitialized data from heap across trust boundary"); + break; + } + if (warned) + { + location_t loc = rich_loc->get_loc (); + inform_number_of_uninit_bits (loc); + complain_about_uninit_ranges (loc); + + if (mem_space == MEMSPACE_STACK) + maybe_emit_fixit_hint (); + } + return warned; + } + + label_text describe_final_event (const evdesc::final_event &) FINAL OVERRIDE + { + enum memory_space mem_space = get_src_memory_space (); + switch (mem_space) + { + default: + return label_text::borrow ("uninitialized data copied here"); + + case MEMSPACE_STACK: + return label_text::borrow ("uninitialized data copied from stack here"); + + case MEMSPACE_HEAP: + return label_text::borrow ("uninitialized data copied from heap here"); + } + } + + void mark_interesting_stuff (interesting_t *interest) FINAL OVERRIDE + { + if (m_src_region) + interest->add_region_creation (m_src_region); + } + +private: + enum memory_space get_src_memory_space () const + { + return m_src_region ? m_src_region->get_memory_space () : MEMSPACE_UNKNOWN; + } + + bit_size_t calc_num_uninit_bits () const + { + switch (m_copied_sval->get_kind ()) + { + default: + gcc_unreachable (); + break; + case SK_POISONED: + { + const poisoned_svalue *poisoned_sval + = as_a (m_copied_sval); + gcc_assert (poisoned_sval->get_poison_kind () == POISON_KIND_UNINIT); + + /* Give up if don't have type information. */ + if (m_copied_sval->get_type () == NULL_TREE) + return 0; + + bit_size_t size_in_bits; + if (int_size_in_bits (m_copied_sval->get_type (), &size_in_bits)) + return size_in_bits; + + /* Give up if we can't get the size of the type. */ + return 0; + } + break; + case SK_COMPOUND: + { + const compound_svalue *compound_sval + = as_a (m_copied_sval); + bit_size_t result = 0; + /* Find keys for uninit svals. */ + for (auto iter : *compound_sval) + { + const svalue *sval = iter.second; + if (const poisoned_svalue *psval + = sval->dyn_cast_poisoned_svalue ()) + if (psval->get_poison_kind () == POISON_KIND_UNINIT) + { + const binding_key *key = iter.first; + const concrete_binding *ckey + = key->dyn_cast_concrete_binding (); + gcc_assert (ckey); + result += ckey->get_size_in_bits (); + } + } + return result; + } + } + } + + void inform_number_of_uninit_bits (location_t loc) const + { + bit_size_t num_uninit_bits = calc_num_uninit_bits (); + if (num_uninit_bits <= 0) + return; + if (num_uninit_bits % BITS_PER_UNIT == 0) + { + /* Express in bytes. */ + byte_size_t num_uninit_bytes = num_uninit_bits / BITS_PER_UNIT; + if (num_uninit_bytes == 1) + inform (loc, "1 byte is uninitialized"); + else + inform (loc, + "%wu bytes are uninitialized", num_uninit_bytes.to_uhwi ()); + } + else + { + /* Express in bits. */ + if (num_uninit_bits == 1) + inform (loc, "1 bit is uninitialized"); + else + inform (loc, + "%wu bits are uninitialized", num_uninit_bits.to_uhwi ()); + } + } + + void complain_about_uninit_ranges (location_t loc) const + { + if (const compound_svalue *compound_sval + = m_copied_sval->dyn_cast_compound_svalue ()) + { + /* Find keys for uninit svals. */ + auto_vec uninit_keys; + for (auto iter : *compound_sval) + { + const svalue *sval = iter.second; + if (const poisoned_svalue *psval + = sval->dyn_cast_poisoned_svalue ()) + if (psval->get_poison_kind () == POISON_KIND_UNINIT) + { + const binding_key *key = iter.first; + const concrete_binding *ckey + = key->dyn_cast_concrete_binding (); + gcc_assert (ckey); + uninit_keys.safe_push (ckey); + } + } + /* Complain about them in sorted order. */ + uninit_keys.qsort (concrete_binding::cmp_ptr_ptr); + + std::unique_ptr layout; + + tree type = m_copied_sval->get_type (); + if (type && TREE_CODE (type) == RECORD_TYPE) + { + // (std::make_unique is C++14) + layout = std::unique_ptr (new record_layout (type)); + + if (0) + layout->dump (); + } + + unsigned i; + const concrete_binding *ckey; + FOR_EACH_VEC_ELT (uninit_keys, i, ckey) + { + bit_offset_t start_bit = ckey->get_start_bit_offset (); + bit_offset_t next_bit = ckey->get_next_bit_offset (); + complain_about_uninit_range (loc, start_bit, next_bit, + layout.get ()); + } + } + } + + void complain_about_uninit_range (location_t loc, + bit_offset_t start_bit, + bit_offset_t next_bit, + const record_layout *layout) const + { + if (layout) + { + while (start_bit < next_bit) + { + if (const record_layout::item *item + = layout->get_item_at (start_bit)) + { + gcc_assert (start_bit >= item->get_start_bit_offset ()); + gcc_assert (start_bit < item->get_next_bit_offset ()); + if (item->get_start_bit_offset () == start_bit + && item->get_next_bit_offset () <= next_bit) + complain_about_fully_uninit_item (*item); + else + complain_about_partially_uninit_item (*item); + start_bit = item->get_next_bit_offset (); + continue; + } + else + break; + } + } + + if (start_bit >= next_bit) + return; + + if (start_bit % 8 == 0 && next_bit % 8 == 0) + { + /* Express in bytes. */ + byte_offset_t start_byte = start_bit / 8; + byte_offset_t last_byte = (next_bit / 8) - 1; + if (last_byte == start_byte) + inform (loc, + "byte %wu is uninitialized", + start_byte.to_uhwi ()); + else + inform (loc, + "bytes %wu - %wu are uninitialized", + start_byte.to_uhwi (), + last_byte.to_uhwi ()); + } + else + { + /* Express in bits. */ + bit_offset_t last_bit = next_bit - 1; + if (last_bit == start_bit) + inform (loc, + "bit %wu is uninitialized", + start_bit.to_uhwi ()); + else + inform (loc, + "bits %wu - %wu are uninitialized", + start_bit.to_uhwi (), + last_bit.to_uhwi ()); + } + } + + static void + complain_about_fully_uninit_item (const record_layout::item &item) + { + tree field = item.m_field; + bit_size_t num_bits = item.m_bit_range.m_size_in_bits; + if (item.m_is_padding) + { + if (num_bits % 8 == 0) + { + /* Express in bytes. */ + byte_size_t num_bytes = num_bits / BITS_PER_UNIT; + if (num_bytes == 1) + inform (DECL_SOURCE_LOCATION (field), + "padding after field %qD is uninitialized (1 byte)", + field); + else + inform (DECL_SOURCE_LOCATION (field), + "padding after field %qD is uninitialized (%wu bytes)", + field, num_bytes.to_uhwi ()); + } + else + { + /* Express in bits. */ + if (num_bits == 1) + inform (DECL_SOURCE_LOCATION (field), + "padding after field %qD is uninitialized (1 bit)", + field); + else + inform (DECL_SOURCE_LOCATION (field), + "padding after field %qD is uninitialized (%wu bits)", + field, num_bits.to_uhwi ()); + } + } + else + { + if (num_bits % 8 == 0) + { + /* Express in bytes. */ + byte_size_t num_bytes = num_bits / BITS_PER_UNIT; + if (num_bytes == 1) + inform (DECL_SOURCE_LOCATION (field), + "field %qD is uninitialized (1 byte)", field); + else + inform (DECL_SOURCE_LOCATION (field), + "field %qD is uninitialized (%wu bytes)", + field, num_bytes.to_uhwi ()); + } + else + { + /* Express in bits. */ + if (num_bits == 1) + inform (DECL_SOURCE_LOCATION (field), + "field %qD is uninitialized (1 bit)", field); + else + inform (DECL_SOURCE_LOCATION (field), + "field %qD is uninitialized (%wu bits)", + field, num_bits.to_uhwi ()); + } + } + } + + static void + complain_about_partially_uninit_item (const record_layout::item &item) + { + tree field = item.m_field; + if (item.m_is_padding) + inform (DECL_SOURCE_LOCATION (field), + "padding after field %qD is partially uninitialized", + field); + else + inform (DECL_SOURCE_LOCATION (field), + "field %qD is partially uninitialized", + field); + /* TODO: ideally we'd describe what parts are uninitialized. */ + } + + void maybe_emit_fixit_hint () const + { + if (tree decl = m_src_region->maybe_get_decl ()) + { + gcc_rich_location hint_richloc (DECL_SOURCE_LOCATION (decl)); + hint_richloc.add_fixit_insert_after (" = {0}"); + inform (&hint_richloc, + "suggest forcing zero-initialization by" + " providing a %<{0}%> initializer"); + } + } + +private: + const region *m_src_region; + const region *m_dest_region; + const svalue *m_copied_sval; + region_model_manager *m_mgr; +}; + +/* Return true if any part of SVAL is uninitialized. */ + +static bool +contains_uninit_p (const svalue *sval) +{ + struct uninit_finder : public visitor + { + public: + uninit_finder () : m_found_uninit (false) {} + void visit_poisoned_svalue (const poisoned_svalue *sval) + { + if (sval->get_poison_kind () == POISON_KIND_UNINIT) + m_found_uninit = true; + } + bool m_found_uninit; + }; + + uninit_finder v; + sval->accept (&v); + + return v.m_found_uninit; +} + +/* Subroutine of region_model::set_value, called when setting DST_REG + when writing through an untrusted pointer (and thus crossing a security + boundary). + + Check that COPIED_SVAL is fully initialized. If not, complain about + an infoleak to CTXT. + + SRC_REG can be NULL; if non-NULL it is used as a hint in the diagnostic + as to where COPIED_SVAL came from. */ + +void +region_model::maybe_complain_about_infoleak (const region *dst_reg, + const svalue *copied_sval, + const region *src_reg, + region_model_context *ctxt) +{ + /* Check for exposure. */ + if (contains_uninit_p (copied_sval)) + ctxt->warn (new exposure_through_uninit_copy (src_reg, + dst_reg, + copied_sval, + m_mgr)); +} + +} // namespace ana + +#endif /* #if ENABLE_ANALYZER */ diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 518a5216c73..e8b939a2fc4 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -438,6 +438,7 @@ Objective-C and Objective-C++ Dialects}. -Wno-analyzer-double-fclose @gol -Wno-analyzer-double-free @gol -Wno-analyzer-exposure-through-output-file @gol +-Wno-analyzer-exposure-through-uninit-copy @gol -Wno-analyzer-file-leak @gol -Wno-analyzer-free-of-non-heap @gol -Wno-analyzer-malloc-leak @gol @@ -9393,6 +9394,7 @@ Enabling this option effectively enables the following warnings: -Wanalyzer-double-fclose @gol -Wanalyzer-double-free @gol -Wanalyzer-exposure-through-output-file @gol +-Wanalyzer-exposure-through-uninit-copy @gol -Wanalyzer-file-leak @gol -Wanalyzer-free-of-non-heap @gol -Wanalyzer-malloc-leak @gol @@ -9461,6 +9463,18 @@ This diagnostic warns for paths through the code in which a security-sensitive value is written to an output file (such as writing a password to a log file). +@item Wanalyzer-exposure-through-uninit-copy +@opindex Wanalyzer-exposure-through-uninit-copy +@opindex Wno-analyzer-exposure-through-uninit-copy +This warning requires @option{-fanalyzer}, which enables it; use +@option{-Wno-analyzer-exposure-through-uninit-copy} +to disable it. + +This diagnostic warns for ``infoleaks'' - paths through the code in which +uninitialized values are copied across a security boundary +(such as copying a partially-initialized struct on the stack +to an untrusted custom address space). + @item -Wno-analyzer-file-leak @opindex Wanalyzer-file-leak @opindex Wno-analyzer-file-leak diff --git a/gcc/testsuite/gcc.dg/analyzer/copy-function-1.c b/gcc/testsuite/gcc.dg/analyzer/copy-function-1.c new file mode 100644 index 00000000000..1b372ac3415 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/copy-function-1.c @@ -0,0 +1,98 @@ +#include "analyzer-decls.h" + +extern void copy_fn_always_succeeds (void *to, const void *from, long n) + __attribute__((access (write_only, 1, 3), + access (read_only, 2, 3) + )); + +extern int copy_fn_zero_on_success (void *to, const void *from, long n) + __attribute__((access (write_only, 1, 3), + access (read_only, 2, 3), + returns_zero_on_success + )); + +extern int copy_fn_zero_on_failure (void *to, const void *from, long n) + __attribute__((access (write_only, 1, 3), + access (read_only, 2, 3), + returns_zero_on_failure + )); + +void +test_1 (int a) +{ + int b; + copy_fn_always_succeeds (&b, &a, sizeof (a)); + __analyzer_eval (a == b); /* { dg-warning "TRUE" } */ +} + +int +test_2 (int a) +{ + int b = 42; + int r = copy_fn_zero_on_success (&b, &a, sizeof (a)); + if (r) + /* Failure. */ + __analyzer_eval (b == 42); /* { dg-warning "TRUE" } */ + else + /* Success. */ + __analyzer_eval (b == a); /* { dg-warning "TRUE" } */ + return r; +} + +int +test_3 (int a) +{ + int b = 42; + int r = copy_fn_zero_on_failure (&b, &a, sizeof (a)); + if (r) + /* Success. */ + __analyzer_eval (b == a); /* { dg-warning "TRUE" } */ + else + /* Failure. */ + __analyzer_eval (b == 42); /* { dg-warning "TRUE" } */ + return r; +} + +/* Different param order. */ + +extern int copy_fn_zero_on_failure_2 (long n, const void *from, void *to) + __attribute__((returns_zero_on_failure, + access (read_only, 2, 1), + access (write_only, 3, 1))); +int +test_4 (int a) +{ + int b = 42; + int r = copy_fn_zero_on_failure_2 (sizeof (a), &a, &b); + if (r) + /* Success. */ + __analyzer_eval (b == a); /* { dg-warning "TRUE" } */ + else + /* Failure. */ + __analyzer_eval (b == 42); /* { dg-warning "TRUE" } */ + return r; +} + +/* Not a copy-fn: too many arguments. */ +extern int too_many_args (void *to, const void *from, long n, int i) + __attribute__((access (write_only, 1, 3), + access (read_only, 2, 3))); + +int test_5 (int a) +{ + int b; + too_many_args (&b, &a, sizeof (a), 17); + __analyzer_eval (a == b); /* { dg-warning "UNKNOWN" } */ +} + +/* Not a copy-fn: variadic. */ +extern int variadic (void *to, const void *from, long n, ...) + __attribute__((access (write_only, 1, 3), + access (read_only, 2, 3))); + +int test_6 (int a) +{ + int b; + variadic (&b, &a, sizeof (a)); + __analyzer_eval (a == b); /* { dg-warning "UNKNOWN" } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/copy_from_user-1.c b/gcc/testsuite/gcc.dg/analyzer/copy_from_user-1.c new file mode 100644 index 00000000000..a1415f38aa6 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/copy_from_user-1.c @@ -0,0 +1,45 @@ +typedef __SIZE_TYPE__ size_t; + +#define __user + +extern int copy_from_user(void *to, const void __user *from, long n) + __attribute__((access (write_only, 1, 3), + access (read_only, 2, 3) + )); + +#define EFAULT 14 +#define EINVAL 22 + +/* Taken from Linux: fs/binfmt_misc.c (GPL-2.0-only). */ + +int parse_command(const char __user *buffer, size_t count) +{ + char s[4]; + + if (count > 3) + return -EINVAL; + if (copy_from_user(s, buffer, count)) + return -EFAULT; + if (!count) + return 0; + if (s[count - 1] == '\n') /* { dg-bogus "uninit" } */ + count--; + if (count == 1 && s[0] == '0') /* { dg-bogus "uninit" } */ + return 1; + if (count == 1 && s[0] == '1') /* { dg-bogus "uninit" } */ + return 2; + if (count == 2 && s[0] == '-' && s[1] == '1') /* { dg-bogus "uninit" } */ + return 3; + return -EINVAL; +} + +/* Not using return value from copy_from_user. */ + +int test_2 (const char __user *buffer, size_t count) +{ + char s[4]; + if (count > 3) + return -EINVAL; + copy_from_user(s, buffer, count); + return 0; +} diff --git a/gcc/testsuite/gcc.dg/analyzer/infoleak-1.c b/gcc/testsuite/gcc.dg/analyzer/infoleak-1.c new file mode 100644 index 00000000000..4b33055c33a --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/infoleak-1.c @@ -0,0 +1,181 @@ +#include + +#include "test-uaccess.h" + +typedef unsigned char u8; +typedef unsigned __INT16_TYPE__ u16; +typedef unsigned __INT32_TYPE__ u32; + +struct s1 +{ + u32 i; +}; + +void test_1a (void __user *dst, u32 a) +{ + struct s1 s; + s.i = a; + copy_to_user(dst, &s, sizeof (struct s1)); /* { dg-bogus "" } */ +} + +void test_1b (void __user *dst, u32 a) +{ + struct s1 s; + copy_to_user(dst, &s, sizeof (struct s1)); /* { dg-warning "potential exposure of sensitive information by copying uninitialized data from stack" "warning" } */ + /* { dg-message "4 bytes are uninitialized" "note how much" { target *-*-* } .-1 } */ +} + +void test_1c (void __user *dst, u32 a) +{ + struct s1 s; + memset (&s, 0, sizeof (struct s1)); + copy_to_user(dst, &s, sizeof (struct s1)); /* { dg-bogus "" } */ +} + +void test_1d (void __user *dst, u32 a) +{ + struct s1 s = {0}; + copy_to_user(dst, &s, sizeof (struct s1)); /* { dg-bogus "" } */ +} + +struct s2 +{ + u32 i; + u32 j; /* { dg-message "field 'j' is uninitialized \\(4 bytes\\)" } */ +}; + +void test_2a (void __user *dst, u32 a) +{ + struct s2 s; /* { dg-message "region created on stack here" "where" } */ + /* { dg-message "capacity: 8 bytes" "capacity" { target *-*-* } .-1 } */ + s.i = a; + copy_to_user(dst, &s, sizeof (struct s2)); /* { dg-warning "potential exposure of sensitive information by copying uninitialized data from stack" "warning" } */ + /* { dg-message "4 bytes are uninitialized" "note how much" { target *-*-* } .-1 } */ +} + +void test_2b (void __user *dst, u32 a) +{ + struct s2 s; + s.i = a; + /* Copy with wrong size (only part of s2). */ + copy_to_user(dst, &s, sizeof (struct s1)); +} + +void test_2d (void __user *dst, u32 a) +{ + struct s2 s = {0}; + s.i = a; + copy_to_user(dst, &s, sizeof (struct s2)); /* { dg-bogus" } */ +} + +struct empty {}; + +void test_empty (void __user *dst) +{ + struct empty e; + copy_to_user(dst, &e, sizeof (struct empty)); +} + +union un_a +{ + u32 i; + u8 j; +}; + +/* As above, but in a different order. */ + +union un_b +{ + u8 j; + u32 i; +}; + +void test_union_1a (void __user *dst, u8 v) +{ + union un_a u; /* { dg-message "region created on stack here" "where" } */ + /* { dg-message "capacity: 4 bytes" "capacity" { target *-*-* } .-1 } */ + u.j = v; + copy_to_user(dst, &u, sizeof (union un_a)); /* { dg-warning "potential exposure of sensitive information by copying uninitialized data from stack" "warning" } */ + /* { dg-message "3 bytes are uninitialized" "note how much" { target *-*-* } .-1 } */ + /* { dg-message "bytes 1 - 3 are uninitialized" "note how much" { target *-*-* } .-2 } */ +} + +void test_union_1b (void __user *dst, u8 v) +{ + union un_b u; /* { dg-message "region created on stack here" "where" } */ + /* { dg-message "capacity: 4 bytes" "capacity" { target *-*-* } .-1 } */ + u.j = v; + copy_to_user(dst, &u, sizeof (union un_b)); /* { dg-warning "potential exposure of sensitive information by copying uninitialized data from stack" "warning" } */ + /* { dg-message "3 bytes are uninitialized" "note how much" { target *-*-* } .-1 } */ + /* { dg-message "bytes 1 - 3 are uninitialized" "note how much" { target *-*-* } .-2 } */ +} + +void test_union_2a (void __user *dst, u8 v) +{ + union un_a u = {0}; + u.j = v; + copy_to_user(dst, &u, sizeof (union un_a)); +} + +void test_union_2b (void __user *dst, u8 v) +{ + union un_b u = {0}; + u.j = v; + copy_to_user(dst, &u, sizeof (union un_b)); +} + +void test_union_3a (void __user *dst, u32 v) +{ + union un_a u; + u.i = v; + copy_to_user(dst, &u, sizeof (union un_a)); /* { dg-bogus "" } */ +} + +void test_union_3b (void __user *dst, u32 v) +{ + union un_b u; + u.i = v; + copy_to_user(dst, &u, sizeof (union un_b)); /* { dg-bogus "" } */ +} + +void test_union_4a (void __user *dst, u8 v) +{ + union un_a u = {0}; + copy_to_user(dst, &u, sizeof (union un_a)); /* { dg-bogus "" } */ +} + +void test_union_4b (void __user *dst, u8 v) +{ + union un_b u = {0}; + copy_to_user(dst, &u, sizeof (union un_b)); /* { dg-bogus "" } */ +} + +struct st_union_5 +{ + union { + u8 f1; + u32 f2; + } u; /* { dg-message "field 'u' is partially uninitialized" } */ +}; + +void test_union_5 (void __user *dst, u8 v) +{ + struct st_union_5 st; /* { dg-message "region created on stack here" "where" } */ + /* { dg-message "capacity: 4 bytes" "capacity" { target *-*-* } .-1 } */ + + /* This write only initializes the u8 within the union "u", + leaving the remaining 3 bytes uninitialized. */ + st.u.f1 = v; + + copy_to_user (dst, &st, sizeof(st)); /* { dg-warning "potential exposure of sensitive information by copying uninitialized data from stack" "warning" } */ + /* { dg-message "3 bytes are uninitialized" "note how much" { target *-*-* } .-1 } */ +} + +void test_one_byte (void __user *dst) +{ + char src; /* { dg-message "region created on stack here" "where" } */ + /* { dg-message "capacity: 1 byte" "capacity" { target *-*-* } .-1 } */ + + copy_to_user (dst, &src, sizeof(src)); /* { dg-warning "potential exposure of sensitive information by copying uninitialized data from stack" "warning" } */ + /* { dg-message "1 byte is uninitialized" "note how much" { target *-*-* } .-1 } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/infoleak-2.c b/gcc/testsuite/gcc.dg/analyzer/infoleak-2.c new file mode 100644 index 00000000000..7510f80a2de --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/infoleak-2.c @@ -0,0 +1,29 @@ +#include + +#include "test-uaccess.h" + +typedef unsigned char u8; +typedef unsigned __INT16_TYPE__ u16; +typedef unsigned __INT32_TYPE__ u32; + +/* Coverage for the various singular and plural forms of bits, bytes, and fields vs padding. */ + +struct st +{ + u32 a; /* { dg-message "field 'a' is uninitialized \\(4 bytes\\)" } */ + int b:1; /* { dg-message "field 'b' is uninitialized \\(1 bit\\)" "field" } */ + /* { dg-message "padding after field 'b' is uninitialized \\(7 bits\\)" "padding" { target *-*-* } .-1 } */ + u8 d; /* { dg-message "field 'd' is uninitialized \\(1 byte\\)" } */ + int c:7; /* { dg-message "padding after field 'c' is uninitialized \\(9 bits\\)" } */ + u16 e; /* { dg-message "padding after field 'e' is uninitialized \\(2 bytes\\)" } */ +}; + +void test (void __user *dst, u16 v) +{ + struct st s; /* { dg-message "region created on stack here" "where" } */ + /* { dg-message "capacity: 12 bytes" "capacity" { target *-*-* } .-1 } */ + /* { dg-message "suggest forcing zero-initialization by providing a '\\{0\\}' initializer" "fix-it" { target *-*-* } .-2 } */ + s.e = v; + copy_to_user(dst, &s, sizeof (struct st)); /* { dg-warning "potential exposure of sensitive information by copying uninitialized data from stack" "warning" } */ + /* { dg-message "10 bytes are uninitialized" "note how much" { target *-*-* } .-1 } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/infoleak-3.c b/gcc/testsuite/gcc.dg/analyzer/infoleak-3.c new file mode 100644 index 00000000000..42dfffa8f65 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/infoleak-3.c @@ -0,0 +1,141 @@ +/* Verify that -Wanalyzer-exposure-through-uninit-copy doesn't get confused + if size argument to copy_to_user is an upper bound, rather than a + constant. */ + +#include "analyzer-decls.h" + +typedef __SIZE_TYPE__ size_t; + +#include "test-uaccess.h" + +typedef unsigned __INT32_TYPE__ u32; + +/* min_t adapted from include/linux/kernel.h. */ + +#define min_t(type, x, y) ({ \ + type __min1 = (x); \ + type __min2 = (y); \ + __min1 < __min2 ? __min1: __min2; }) + +struct st +{ + u32 a; + u32 b; +}; + +/* Verify that we cope with min_t. */ + +void test_1_full_init (void __user *dst, u32 x, u32 y, unsigned long in_sz) +{ + struct st s; + s.a = x; + s.b = y; + unsigned long copy_sz = min_t(unsigned long, in_sz, sizeof(s)); + copy_to_user(dst, &s, copy_sz); /* { dg-bogus "exposure" } */ +} + +void test_1_partial_init (void __user *dst, u32 x, u32 y, unsigned long in_sz) +{ + struct st s; + s.a = x; + /* s.y not initialized. */ + unsigned long copy_sz = min_t(unsigned long, in_sz, sizeof(s)); + copy_to_user(dst, &s, copy_sz); /* { dg-warning "exposure" } */ +} + +/* Constant on LHS rather than RHS. */ + +void test_2_full_init (void __user *dst, u32 x, u32 y, unsigned long in_sz) +{ + struct st s; + s.a = x; + s.b = y; + unsigned long copy_sz = min_t(unsigned long, sizeof(s), in_sz); + copy_to_user(dst, &s, copy_sz); /* { dg-bogus "exposure" } */ +} + +void test_2_partial_init (void __user *dst, u32 x, u32 y, unsigned long in_sz) +{ + struct st s; + s.a = x; + /* s.y not initialized. */ + unsigned long copy_sz = min_t(unsigned long, sizeof(s), in_sz); + copy_to_user(dst, &s, copy_sz); /* { dg-warning "exposure" } */ +} + +/* min_t with various casts. */ + +void test_3_full_init (void __user *dst, u32 x, u32 y, int in_sz) +{ + struct st s; + s.a = x; + s.b = y; + int copy_sz = min_t(unsigned int, in_sz, sizeof(s)); + copy_to_user(dst, &s, copy_sz); /* { dg-bogus "exposure" } */ +} + +void test_3_partial_init (void __user *dst, u32 x, u32 y, int in_sz) +{ + struct st s; + s.a = x; + /* s.y not initialized. */ + int copy_sz = min_t(unsigned int, in_sz, sizeof(s)); + copy_to_user(dst, &s, copy_sz); /* { dg-warning "exposure" } */ +} + +/* Comparison against an upper bound. */ + +void test_4_full_init (void __user *dst, u32 x, u32 y, size_t in_sz) +{ + struct st s; + s.a = x; + s.b = y; + + size_t copy_sz = in_sz; + if (copy_sz > sizeof(s)) + copy_sz = sizeof(s); + + copy_to_user(dst, &s, copy_sz); /* { dg-bogus "exposure" } */ +} + +void test_4_partial_init (void __user *dst, u32 x, u32 y, size_t in_sz) +{ + struct st s; + s.a = x; + /* s.y not initialized. */ + + size_t copy_sz = in_sz; + if (copy_sz > sizeof(s)) + copy_sz = sizeof(s); + + copy_to_user(dst, &s, copy_sz); /* { dg-warning "exposure" } */ +} + +/* Comparison against an upper bound with casts. */ + +void test_5_full_init (void __user *dst, u32 x, u32 y, int in_sz) +{ + struct st s; + s.a = x; + s.b = y; + + int copy_sz = in_sz; + if (copy_sz > sizeof(s)) + copy_sz = sizeof(s); + copy_to_user(dst, &s, copy_sz); /* { dg-bogus "exposure" } */ +} + +/* Comparison against an upper bound with casts. */ + +void test_5_partial_init (void __user *dst, u32 x, u32 y, int in_sz) +{ + struct st s; + s.a = x; + /* s.y not initialized. */ + + int copy_sz = in_sz; + if (copy_sz > sizeof(s)) + copy_sz = sizeof(s); + + copy_to_user(dst, &s, copy_sz); /* { dg-warning "exposure" "" } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/infoleak-5.c b/gcc/testsuite/gcc.dg/analyzer/infoleak-5.c new file mode 100644 index 00000000000..94ebb62062b --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/infoleak-5.c @@ -0,0 +1,35 @@ +#include "test-uaccess.h" + +typedef unsigned char u8; +typedef unsigned __INT16_TYPE__ u16; +typedef unsigned __INT32_TYPE__ u32; + +/* As per infoleak-1.c, but doing it here via deref assignment, + rather than copy_to_user. */ + +struct s1 +{ + u32 i; +}; + +void test_1a (struct s1 __user *dst, u32 a) +{ + struct s1 s; + s.i = a; + *dst = s; +} + +union un_a +{ + u32 i; + u8 j; +}; + +void test_union_1a (union un_a __user *dst, u8 v) +{ + union un_a u; + u.j = v; + *dst = u; /* { dg-warning "potential exposure of sensitive information by copying uninitialized data across trust boundary" "warning" } */ + /* { dg-message "3 bytes are uninitialized" "note how much" { target *-*-* } .-1 } */ + /* { dg-message "bytes 1 - 3 are uninitialized" "note how much" { target *-*-* } .-2 } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/infoleak-CVE-2011-1078-1.c b/gcc/testsuite/gcc.dg/analyzer/infoleak-CVE-2011-1078-1.c new file mode 100644 index 00000000000..c3b22320e91 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/infoleak-CVE-2011-1078-1.c @@ -0,0 +1,134 @@ +/* "The sco_sock_getsockopt_old function in net/bluetooth/sco.c in the + Linux kernel before 2.6.39 does not initialize a certain structure, + which allows local users to obtain potentially sensitive information + from kernel stack memory via the SCO_CONNINFO option." + + Fixed e.g. by c4c896e1471aec3b004a693c689f60be3b17ac86 on linux-2.6.39.y + in linux-stable. */ + +#include + +typedef unsigned char __u8; +typedef unsigned short __u16; + +#include "test-uaccess.h" + +/* Adapted from include/asm-generic/uaccess.h. */ + +#define get_user(x, ptr) \ +({ \ + /* [...snip...] */ \ + __get_user_fn(sizeof (*(ptr)), ptr, &(x)); \ + /* [...snip...] */ \ +}) + +static inline int __get_user_fn(size_t size, const void __user *ptr, void *x) +{ + size = copy_from_user(x, ptr, size); + return size ? -1 : size; +} + +/* Adapted from include/linux/kernel.h. */ + +#define min_t(type, x, y) ({ \ + type __min1 = (x); \ + type __min2 = (y); \ + __min1 < __min2 ? __min1: __min2; }) + +/* Adapted from include/linux/net.h. */ + +struct socket { + /* [...snip...] */ + struct sock *sk; + /* [...snip...] */ +}; + +/* Adapted from include/net/bluetooth/sco.h. */ + +struct sco_conninfo { + __u16 hci_handle; + __u8 dev_class[3]; /* { dg-message "padding after field 'dev_class' is uninitialized \\(1 byte\\)" } */ +}; + +struct sco_conn { + + struct hci_conn *hcon; + /* [...snip...] */ +}; + +#define sco_pi(sk) ((struct sco_pinfo *) sk) + +struct sco_pinfo { + /* [...snip...] */ + struct sco_conn *conn; +}; + +/* Adapted from include/net/bluetooth/hci_core.h. */ + +struct hci_conn { + /* [...snip...] */ + __u16 handle; + /* [...snip...] */ + __u8 dev_class[3]; + /* [...snip...] */ +}; + +/* Adapted from sco_sock_getsockopt_old in net/bluetooth/sco.c. */ + +static int sco_sock_getsockopt_old_broken(struct socket *sock, int optname, char __user *optval, int __user *optlen) +{ + struct sock *sk = sock->sk; + /* [...snip...] */ + struct sco_conninfo cinfo; /* { dg-message "region created on stack here" "where" } */ + /* { dg-message "capacity: 6 bytes" "capacity" { target *-*-* } .-1 } */ + /* Note: 40 bits of fields, padded to 48. */ + + int len, err = 0; + + /* [...snip...] */ + + if (get_user(len, optlen)) + return -1; + + /* [...snip...] */ + + /* case SCO_CONNINFO: */ + cinfo.hci_handle = sco_pi(sk)->conn->hcon->handle; + memcpy(cinfo.dev_class, sco_pi(sk)->conn->hcon->dev_class, 3); + + len = min_t(unsigned int, len, sizeof(cinfo)); + if (copy_to_user(optval, (char *)&cinfo, len)) /* { dg-warning "potential exposure of sensitive information by copying uninitialized data from stack" "warning" { target *-*-* } } */ + /* { dg-message "1 byte is uninitialized" "how much note" { target *-*-* } .-1 } */ + err = -1; + + /* [...snip...] */ +} + +static int sco_sock_getsockopt_fixed(struct socket *sock, int optname, char __user *optval, int __user *optlen) +{ + struct sock *sk = sock->sk; + /* [...snip...] */ + struct sco_conninfo cinfo; + /* Note: 40 bits of fields, padded to 48. */ + + int len, err = 0; + + /* [...snip...] */ + + if (get_user(len, optlen)) + return -1; + + /* [...snip...] */ + + /* case SCO_CONNINFO: */ + /* Infoleak fixed by this memset call. */ + memset(&cinfo, 0, sizeof(cinfo)); + cinfo.hci_handle = sco_pi(sk)->conn->hcon->handle; + memcpy(cinfo.dev_class, sco_pi(sk)->conn->hcon->dev_class, 3); + + len = min_t(unsigned int, len, sizeof(cinfo)); + if (copy_to_user(optval, (char *)&cinfo, len)) /* { dg-bogus "exposure" } */ + err = -1; + + /* [...snip...] */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/infoleak-CVE-2011-1078-2.c b/gcc/testsuite/gcc.dg/analyzer/infoleak-CVE-2011-1078-2.c new file mode 100644 index 00000000000..811f5f6c605 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/infoleak-CVE-2011-1078-2.c @@ -0,0 +1,42 @@ +/* Simplified versions of infoleak-CVE-2011-1078-1.c. */ + +#include + +typedef unsigned char __u8; +typedef unsigned short __u16; + +#include "test-uaccess.h" + +/* Adapted from include/net/bluetooth/sco.h. */ + +struct sco_conninfo { + __u16 hci_handle; + __u8 dev_class[3]; /* { dg-message "padding after field 'dev_class' is uninitialized \\(1 byte\\)" } */ +}; + +/* Adapted from sco_sock_getsockopt_old in net/bluetooth/sco.c. */ + +int test_1 (char __user *optval, const struct sco_conninfo *in) +{ + struct sco_conninfo cinfo; /* { dg-message "region created on stack here" "where" } */ + /* { dg-message "capacity: 6 bytes" "capacity" { target *-*-* } .-1 } */ + /* Note: 40 bits of fields, padded to 48. */ + + cinfo.hci_handle = in->hci_handle; + memcpy(cinfo.dev_class, in->dev_class, 3); + + copy_to_user(optval, &cinfo, sizeof(cinfo)); /* { dg-warning "potential exposure of sensitive information by copying uninitialized data from stack" "warning" } */ + /* { dg-message "1 byte is uninitialized" "how much note" { target *-*-* } .-1 } */ +} + +int test_2 (char __user *optval, const struct sco_conninfo *in) +{ + struct sco_conninfo cinfo; + /* Note: 40 bits of fields, padded to 48. */ + + memset(&cinfo, 0, sizeof(cinfo)); + cinfo.hci_handle = in->hci_handle; + memcpy(cinfo.dev_class, in->dev_class, 3); + + copy_to_user(optval, &cinfo, sizeof(cinfo)); /* { dg-bogus "" } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/infoleak-CVE-2014-1446-1.c b/gcc/testsuite/gcc.dg/analyzer/infoleak-CVE-2014-1446-1.c new file mode 100644 index 00000000000..2726a9c0f38 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/infoleak-CVE-2014-1446-1.c @@ -0,0 +1,117 @@ +/* "The yam_ioctl function in drivers/net/hamradio/yam.c in the Linux kernel + before 3.12.8 does not initialize a certain structure member, which allows + local users to obtain sensitive information from kernel memory by + leveraging the CAP_NET_ADMIN capability for an SIOCYAMGCFG ioctl call." + + Fixed e.g. by e7834c71c2cacc621ddc64bd71f83ef2054f6539 on linux-3.12.y + in linux-stable. */ + +#include + +#include "test-uaccess.h" + +/* Adapted from include/linux/yam.h */ + +struct yamcfg { + unsigned int mask; /* Mask of commands */ + unsigned int iobase; /* IO Base of COM port */ + unsigned int irq; /* IRQ of COM port */ + unsigned int bitrate; /* Bit rate of radio port */ + unsigned int baudrate; /* Baud rate of the RS232 port */ + unsigned int txdelay; /* TxDelay */ + unsigned int txtail; /* TxTail */ + unsigned int persist; /* Persistence */ + unsigned int slottime; /* Slottime */ + unsigned int mode; /* mode 0 (simp), 1(Dupl), 2(Dupl+delay) */ + unsigned int holddly; /* PTT delay in FullDuplex 2 mode */ +}; + +struct yamdrv_ioctl_cfg { + int cmd; /* { dg-message "field 'cmd' is uninitialized \\(4 bytes\\)" } */ + struct yamcfg cfg; +}; + +/* Adapted from include/asm-generic/errno-base.h */ + +#define EFAULT 14 /* Bad address */ + +/* Adapted from drivers/net/hamradio/yam.c */ + +struct yam_port { + /* [...snip...] */ + + int bitrate; + int baudrate; + int iobase; + int irq; + int dupmode; + + /* [...snip...] */ + + int txd; /* tx delay */ + int holdd; /* duplex ptt delay */ + int txtail; /* txtail delay */ + int slot; /* slottime */ + int pers; /* persistence */ + + /* [...snip...] */ +}; + +/* Broken version, leaving yi.cmd uninitialized. */ + +static int yam_ioctl(/* [...snip...] */ + void __user *dst, struct yam_port *yp) +{ + struct yamdrv_ioctl_cfg yi; /* { dg-message "region created on stack here" "memspace event" } */ + /* { dg-message "capacity: 48 bytes" "capacity event" { target *-*-* } .-1 } */ + + /* [...snip...] */ + + /* case SIOCYAMGCFG: */ + yi.cfg.mask = 0xffffffff; + yi.cfg.iobase = yp->iobase; + yi.cfg.irq = yp->irq; + yi.cfg.bitrate = yp->bitrate; + yi.cfg.baudrate = yp->baudrate; + yi.cfg.mode = yp->dupmode; + yi.cfg.txdelay = yp->txd; + yi.cfg.holddly = yp->holdd; + yi.cfg.txtail = yp->txtail; + yi.cfg.persist = yp->pers; + yi.cfg.slottime = yp->slot; + if (copy_to_user(dst, &yi, sizeof(struct yamdrv_ioctl_cfg))) /* { dg-warning "potential exposure of sensitive information by copying uninitialized data from stack" "warning" } */ + /* { dg-message "4 bytes are uninitialized" "how much note" { target *-*-* } .-1 } */ + return -EFAULT; + /* [...snip...] */ + + return 0; +} + +/* Fixed version, with a memset. */ + +static int yam_ioctl_fixed(/* [...snip...] */ + void __user *dst, struct yam_port *yp) +{ + struct yamdrv_ioctl_cfg yi; + + /* [...snip...] */ + + /* case SIOCYAMGCFG: */ + memset(&yi, 0, sizeof(yi)); + yi.cfg.mask = 0xffffffff; + yi.cfg.iobase = yp->iobase; + yi.cfg.irq = yp->irq; + yi.cfg.bitrate = yp->bitrate; + yi.cfg.baudrate = yp->baudrate; + yi.cfg.mode = yp->dupmode; + yi.cfg.txdelay = yp->txd; + yi.cfg.holddly = yp->holdd; + yi.cfg.txtail = yp->txtail; + yi.cfg.persist = yp->pers; + yi.cfg.slottime = yp->slot; + if (copy_to_user(dst, &yi, sizeof(struct yamdrv_ioctl_cfg))) /* { dg-bogus "" } */ + return -EFAULT; + /* [...snip...] */ + + return 0; +} diff --git a/gcc/testsuite/gcc.dg/analyzer/infoleak-CVE-2017-18549-1.c b/gcc/testsuite/gcc.dg/analyzer/infoleak-CVE-2017-18549-1.c new file mode 100644 index 00000000000..d40effb1cc6 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/infoleak-CVE-2017-18549-1.c @@ -0,0 +1,101 @@ +/* "An issue was discovered in drivers/scsi/aacraid/commctrl.c in the + Linux kernel before 4.13. There is potential exposure of kernel stack + memory because aac_send_raw_srb does not initialize the reply structure." + + Fixed e.g. by 342ffc26693b528648bdc9377e51e4f2450b4860 on linux-4.13.y + in linux-stable. + + This is a very simplified version of that code (before and after the fix). */ + +#include + +typedef unsigned int __u32; +typedef unsigned int u32; +typedef unsigned char u8; + +#include "test-uaccess.h" + +/* Adapted from include/uapi/linux/types.h */ + +#define __bitwise +typedef __u32 __bitwise __le32; + +/* Adapted from drivers/scsi/aacraid/aacraid.h */ + +#define AAC_SENSE_BUFFERSIZE 30 + +struct aac_srb_reply +{ + __le32 status; + __le32 srb_status; + __le32 scsi_status; + __le32 data_xfer_length; + __le32 sense_data_size; + u8 sense_data[AAC_SENSE_BUFFERSIZE]; /* { dg-message "padding after field 'sense_data' is uninitialized \\(2 bytes\\)" } */ +}; + +#define ST_OK 0 +#define SRB_STATUS_SUCCESS 0x01 + +/* Adapted from drivers/scsi/aacraid/commctrl.c */ + +static int aac_send_raw_srb(/* [...snip...] */ + void __user *user_reply) +{ + u32 byte_count = 0; + + /* [...snip...] */ + + struct aac_srb_reply reply; /* { dg-message "region created on stack here" "memspace message" } */ + /* { dg-message "capacity: 52 bytes" "capacity message" { target *-*-* } .-1 } */ + + reply.status = ST_OK; + + /* [...snip...] */ + + reply.srb_status = SRB_STATUS_SUCCESS; + reply.scsi_status = 0; + reply.data_xfer_length = byte_count; + reply.sense_data_size = 0; + memset(reply.sense_data, 0, AAC_SENSE_BUFFERSIZE); + + /* [...snip...] */ + + if (copy_to_user(user_reply, &reply, /* { dg-warning "potential exposure of sensitive information by copying uninitialized data from stack" } */ + /* { dg-message "2 bytes are uninitialized" "note how much" { target *-*-* } .-1 } */ + sizeof(struct aac_srb_reply))) { + /* [...snip...] */ + } + /* [...snip...] */ +} + +static int aac_send_raw_srb_fixed(/* [...snip...] */ + void __user *user_reply) +{ + u32 byte_count = 0; + + /* [...snip...] */ + + struct aac_srb_reply reply; + + /* This is the fix. */ + memset(&reply, 0, sizeof(reply)); + + reply.status = ST_OK; + + /* [...snip...] */ + + reply.srb_status = SRB_STATUS_SUCCESS; + reply.scsi_status = 0; + reply.data_xfer_length = byte_count; + reply.sense_data_size = 0; + memset(reply.sense_data, 0, AAC_SENSE_BUFFERSIZE); + + /* [...snip...] */ + + if (copy_to_user(user_reply, &reply, /* { dg-bogus "" } */ + sizeof(struct aac_srb_reply))) { + /* [...snip...] */ + } + /* [...snip...] */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/infoleak-CVE-2017-18550-1.c b/gcc/testsuite/gcc.dg/analyzer/infoleak-CVE-2017-18550-1.c new file mode 100644 index 00000000000..426efe037df --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/infoleak-CVE-2017-18550-1.c @@ -0,0 +1,171 @@ +/* "An issue was discovered in drivers/scsi/aacraid/commctrl.c in the + Linux kernel before 4.13. There is potential exposure of kernel stack + memory because aac_get_hba_info does not initialize the hbainfo structure." + + Fixed e.g. by 342ffc26693b528648bdc9377e51e4f2450b4860 on linux-4.13.y + in linux-stable. + + This is a simplified version of that code (before and after the fix). */ + +#include + +typedef unsigned int __u32; +typedef unsigned int u32; +typedef unsigned char u8; + +#include "test-uaccess.h" + +/* Adapted from include/uapi/linux/types.h */ + +#define __bitwise +typedef __u32 __bitwise __le32; + +/* Adapted from drivers/scsi/aacraid/aacraid.h */ + +struct aac_hba_info { + + u8 driver_name[50]; /* { dg-message "field 'driver_name' is uninitialized \\(50 bytes\\)" } */ + u8 adapter_number; + u8 system_io_bus_number; + u8 device_number; /* { dg-message "padding after field 'device_number' is uninitialized \\(3 bytes\\)" } */ + u32 function_number; + u32 vendor_id; + u32 device_id; + u32 sub_vendor_id; + u32 sub_system_id; + u32 mapped_base_address_size; /* { dg-message "field 'mapped_base_address_size' is uninitialized \\(4 bytes\\)" } */ + u32 base_physical_address_high_part; + u32 base_physical_address_low_part; + + u32 max_command_size; + u32 max_fib_size; + u32 max_scatter_gather_from_os; + u32 max_scatter_gather_to_fw; + u32 max_outstanding_fibs; + + u32 queue_start_threshold; + u32 queue_dump_threshold; + u32 max_io_size_queued; + u32 outstanding_io; + + u32 firmware_build_number; + u32 bios_build_number; + u32 driver_build_number; + u32 serial_number_high_part; + u32 serial_number_low_part; + u32 supported_options; + u32 feature_bits; + u32 currentnumber_ports; + + u8 new_comm_interface:1; /* { dg-message "field 'new_comm_interface' is uninitialized \\(1 bit\\)" } */ + u8 new_commands_supported:1; + u8 disable_passthrough:1; + u8 expose_non_dasd:1; + u8 queue_allowed:1; + u8 bled_check_enabled:1; + u8 reserved1:1; + u8 reserted2:1; + + u32 reserved3[10]; /* { dg-message "field 'reserved3' is uninitialized \\(40 bytes\\)" } */ + +}; + +struct aac_dev +{ + /* [...snip...] */ + int id; + /* [...snip...] */ + struct pci_dev *pdev; /* Our PCI interface */ + /* [...snip...] */ +}; + +/* Adapted from include/linux/pci.h */ + +struct pci_dev { + /* [...snip...] */ + struct pci_bus *bus; /* bus this device is on */ + /* [...snip...] */ + unsigned int devfn; /* encoded device & function index */ + unsigned short vendor; + unsigned short device; + unsigned short subsystem_vendor; + unsigned short subsystem_device; + /* [...snip...] */ +}; + +struct pci_bus { + /* [...snip...] */ + unsigned char number; /* bus number */ + /* [...snip...] */ +}; + +/* Adapted from drivers/scsi/aacraid/commctrl.c */ + +static int aac_get_hba_info(struct aac_dev *dev, void __user *arg) +{ + struct aac_hba_info hbainfo; /* { dg-message "region created on stack here" "memspace message" } */ + /* { dg-message "capacity: 200 bytes" "capacity message" { target *-*-* } .-1 } */ + + hbainfo.adapter_number = (u8) dev->id; + hbainfo.system_io_bus_number = dev->pdev->bus->number; + hbainfo.device_number = (dev->pdev->devfn >> 3); + hbainfo.function_number = (dev->pdev->devfn & 0x0007); + + hbainfo.vendor_id = dev->pdev->vendor; + hbainfo.device_id = dev->pdev->device; + hbainfo.sub_vendor_id = dev->pdev->subsystem_vendor; + hbainfo.sub_system_id = dev->pdev->subsystem_device; + + if (copy_to_user(arg, &hbainfo, sizeof(struct aac_hba_info))) { /* { dg-warning "potential exposure of sensitive information by copying uninitialized data from stack" "warning" } */ + /* { dg-message "177 bytes are uninitialized" "how much" { target *-*-* } .-1 } */ + /* [...snip...] */ + } + + return 0; +} + +static int aac_get_hba_info_fixed(struct aac_dev *dev, void __user *arg) +{ + struct aac_hba_info hbainfo; + + memset(&hbainfo, 0, sizeof(hbainfo)); + hbainfo.adapter_number = (u8) dev->id; + hbainfo.system_io_bus_number = dev->pdev->bus->number; + hbainfo.device_number = (dev->pdev->devfn >> 3); + hbainfo.function_number = (dev->pdev->devfn & 0x0007); + + hbainfo.vendor_id = dev->pdev->vendor; + hbainfo.device_id = dev->pdev->device; + hbainfo.sub_vendor_id = dev->pdev->subsystem_vendor; + hbainfo.sub_system_id = dev->pdev->subsystem_device; + + if (copy_to_user(arg, &hbainfo, sizeof(struct aac_hba_info))) { /* { dg-bogus "" } */ + /* [...snip...] */ + } + + return 0; +} + +/* An alternate fix using "= {0}" rather than memset. */ + +static int aac_get_hba_info_fixed_alt(struct aac_dev *dev, void __user *arg) +{ + struct aac_hba_info hbainfo = {0}; + + memset(&hbainfo, 0, sizeof(hbainfo)); + hbainfo.adapter_number = (u8) dev->id; + hbainfo.system_io_bus_number = dev->pdev->bus->number; + hbainfo.device_number = (dev->pdev->devfn >> 3); + hbainfo.function_number = (dev->pdev->devfn & 0x0007); + + hbainfo.vendor_id = dev->pdev->vendor; + hbainfo.device_id = dev->pdev->device; + hbainfo.sub_vendor_id = dev->pdev->subsystem_vendor; + hbainfo.sub_system_id = dev->pdev->subsystem_device; + + if (copy_to_user(arg, &hbainfo, sizeof(struct aac_hba_info))) { /* { dg-bogus "" } */ + /* [...snip...] */ + } + + return 0; +} diff --git a/gcc/testsuite/gcc.dg/analyzer/infoleak-antipatterns-1.c b/gcc/testsuite/gcc.dg/analyzer/infoleak-antipatterns-1.c new file mode 100644 index 00000000000..449a4266b2e --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/infoleak-antipatterns-1.c @@ -0,0 +1,162 @@ +/* Adapted and simplified decls from linux kernel headers. */ + +typedef unsigned char u8; +typedef unsigned __INT16_TYPE__ u16; +typedef unsigned __INT32_TYPE__ u32; +typedef __SIZE_TYPE__ size_t; + +#define EFAULT 14 + +#include "test-uaccess.h" + +typedef unsigned int gfp_t; +#define GFP_KERNEL 0 + +void kfree(const void *); +void *kmalloc(size_t size, gfp_t flags) + __attribute__((malloc (kfree))); + +/* Adapted from antipatterns.ko:infoleak.c (GPL-v2.0). */ + +struct infoleak_buf +{ + char buf[256]; +}; + +int infoleak_stack_no_init(void __user *dst) +{ + struct infoleak_buf st; /* { dg-message "region created on stack here" "where" } */ + /* { dg-message "capacity: 256 bytes" "capacity" { target *-*-* } .-1 } */ + + /* No initialization of "st" at all. */ + if (copy_to_user(dst, &st, sizeof(st))) /* { dg-warning "potential exposure of sensitive information by copying uninitialized data from stack" "warning" } */ + /* { dg-message "256 bytes are uninitialized" "note how much" { target *-*-* } .-1 } */ + return -EFAULT; + return 0; +} + +int infoleak_heap_no_init(void __user *dst) +{ + struct infoleak_buf *heapbuf = kmalloc(sizeof(*heapbuf), GFP_KERNEL); + /* No initialization of "heapbuf" at all. */ + + /* TODO: we also don't check that heapbuf could be NULL when copying + from it. */ + if (copy_to_user(dst, heapbuf, sizeof(*heapbuf))) /* { dg-warning "exposure" "warning" { xfail *-*-* } } */ + /* TODO(xfail). */ + return -EFAULT; /* { dg-warning "leak of 'heapbuf'" } */ + + kfree(heapbuf); + return 0; +} + +struct infoleak_2 +{ + u32 a; + u32 b; /* { dg-message "field 'b' is uninitialized \\(4 bytes\\)" } */ +}; + +int infoleak_stack_missing_a_field(void __user *dst, u32 v) +{ + struct infoleak_2 st; /* { dg-message "region created on stack here" "where" } */ + /* { dg-message "capacity: 8 bytes" "capacity" { target *-*-* } .-1 } */ + + st.a = v; + /* No initialization of "st.b". */ + if (copy_to_user(dst, &st, sizeof(st))) /* { dg-warning "potential exposure of sensitive information by copying uninitialized data from stack" "warning" } */ + /* { dg-message "4 bytes are uninitialized" "note how much" { target *-*-* } .-1 } */ + return -EFAULT; + return 0; +} + +int infoleak_heap_missing_a_field(void __user *dst, u32 v) +{ + struct infoleak_2 *heapbuf = kmalloc(sizeof(*heapbuf), GFP_KERNEL); + heapbuf->a = v; /* { dg-warning "dereference of possibly-NULL 'heapbuf'" } */ + /* No initialization of "heapbuf->b". */ + if (copy_to_user(dst, heapbuf, sizeof(*heapbuf))) /* { dg-warning "exposure" "warning" { xfail *-*-* } } */ + /* TODO(xfail). */ + { + kfree(heapbuf); + return -EFAULT; + } + kfree(heapbuf); + return 0; +} + +struct infoleak_3 +{ + u8 a; /* { dg-message "padding after field 'a' is uninitialized \\(3 bytes\\)" } */ + /* padding here */ + u32 b; +}; + +int infoleak_stack_padding(void __user *dst, u8 p, u32 q) +{ + struct infoleak_3 st; /* { dg-message "region created on stack here" "where" } */ + /* { dg-message "capacity: 8 bytes" "capacity" { target *-*-* } .-1 } */ + + st.a = p; + st.b = q; + /* No initialization of padding. */ + if (copy_to_user(dst, &st, sizeof(st))) /* { dg-warning "potential exposure of sensitive information by copying uninitialized data from stack" "warning" } */ + /* { dg-message "3 bytes are uninitialized" "note how much" { target *-*-* } .-1 } */ + return -EFAULT; + return 0; +} + +int infoleak_stack_unchecked_err(void __user *dst, void __user *src) +{ + struct infoleak_buf st; /* { dg-message "region created on stack here" "where" } */ + /* { dg-message "capacity: 256 bytes" "capacity" { target *-*-* } .-1 } */ + + /* + * If the copy_from_user call fails, then st is still uninitialized, + * and if the copy_to_user call succeds, we have an infoleak. + */ + int err = copy_from_user (&st, src, sizeof(st)); /* { dg-message "when 'copy_from_user' fails" } */ + err |= copy_to_user (dst, &st, sizeof(st)); /* { dg-warning "exposure" "warning" } */ + /* { dg-message "256 bytes are uninitialized" "note how much" { target *-*-* } .-1 } */ + /* Actually, it's *up to* 256 bytes. */ + + if (err) + return -EFAULT; + return 0; +} + +struct infoleak_4 +{ + union { + u8 f1; + u32 f2; + } u; +}; + +int infoleak_stack_union(void __user *dst, u8 v) +{ + struct infoleak_4 st; + /* + * This write only initializes the u8 within the union "u", + * leaving the remaining 3 bytes uninitialized. + */ + st.u.f1 = v; + if (copy_to_user(dst, &st, sizeof(st))) /* { dg-warning "potential exposure of sensitive information by copying uninitialized data from stack" "warning" } */ + /* { dg-message "3 bytes are uninitialized" "note how much" { target *-*-* } .-1 } */ + return -EFAULT; + return 0; +} + +struct infoleak_5 +{ + void *ptr; +}; + +int infoleak_stack_kernel_ptr(void __user *dst, void *kp) +{ + struct infoleak_5 st; + /* This writes a kernel-space pointer into a user space buffer. */ + st.ptr = kp; + if (copy_to_user(dst, &st, sizeof(st))) // TODO: we don't complain about this yet + return -EFAULT; + return 0; +} diff --git a/gcc/testsuite/gcc.dg/analyzer/infoleak-fixit-1.c b/gcc/testsuite/gcc.dg/analyzer/infoleak-fixit-1.c new file mode 100644 index 00000000000..eb04fea8ac9 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/infoleak-fixit-1.c @@ -0,0 +1,22 @@ +#include + +#include "test-uaccess.h" + +typedef unsigned char u8; +typedef unsigned int u32; + +struct st +{ + u8 i; /* { dg-message "padding after field 'i' is uninitialized \\(3 bytes\\)" } */ + u32 j; /* { dg-message "field 'j' is uninitialized \\(4 bytes\\)" } */ +}; + +void test (void __user *dst, u8 a) +{ + struct st s; /* { dg-message "region created on stack here" "where" } */ + /* { dg-message "capacity: 8 bytes" "capacity" { target *-*-* } .-1 } */ + /* { dg-message "suggest forcing zero-initialization by providing a '.0.' initializer" "fix-it hint" { target *-*-* } .-2 } */ + s.i = a; + copy_to_user(dst, &s, sizeof (struct st)); /* { dg-warning "potential exposure of sensitive information by copying uninitialized data from stack" "warning" } */ + /* { dg-message "7 bytes are uninitialized" "note how much" { target *-*-* } .-1 } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/torture/infoleak-net-ethtool-ioctl.c b/gcc/testsuite/gcc.dg/analyzer/torture/infoleak-net-ethtool-ioctl.c new file mode 100644 index 00000000000..20dfb9f3016 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/torture/infoleak-net-ethtool-ioctl.c @@ -0,0 +1,78 @@ +/* Reduced from infoleak false positive seen on Linux kernel with + net/ethtool/ioctl.c */ + +typedef signed char __s8; +typedef unsigned char __u8; +typedef unsigned int __u32; +typedef __s8 s8; +typedef __u32 u32; +enum { false = 0, true = 1 }; +typedef unsigned long __kernel_ulong_t; +typedef __kernel_ulong_t __kernel_size_t; +typedef _Bool bool; +typedef __kernel_size_t size_t; + +void *memset(void *s, int c, size_t n); + +extern bool +check_copy_size(const void *addr, size_t bytes, bool is_source); +extern unsigned long +_copy_from_user(void *, const void *, unsigned long); +extern unsigned long +_copy_to_user(void *, const void *, unsigned long); + +static inline +__attribute__((__always_inline__)) unsigned long +copy_from_user(void *to, const void *from, unsigned long n) { + if (__builtin_expect(!!(check_copy_size(to, n, false)), 1)) + n = _copy_from_user(to, from, n); + return n; +} +static inline +__attribute__((__always_inline__)) unsigned long +copy_to_user(void *to, const void *from, unsigned long n) { + if (__builtin_expect(!!(check_copy_size(from, n, true)), 1)) + n = _copy_to_user(to, from, n); + return n; +} +enum ethtool_link_mode_bit_indices { + __ETHTOOL_LINK_MODE_MASK_NBITS = 92 +}; +struct ethtool_link_settings { + __u32 cmd; + /* [...snip...] */ + __s8 link_mode_masks_nwords; + /* [...snip...] */ +}; + +struct ethtool_link_ksettings { + struct ethtool_link_settings base; + u32 lanes; +}; + +int ethtool_get_link_ksettings(void *useraddr) { + int err = 0; + struct ethtool_link_ksettings link_ksettings; + + if (copy_from_user(&link_ksettings.base, useraddr, + sizeof(link_ksettings.base))) + return -14; + + if ((((__ETHTOOL_LINK_MODE_MASK_NBITS) + (32) - 1) / (32)) != + link_ksettings.base.link_mode_masks_nwords) { + + memset(&link_ksettings, 0, sizeof(link_ksettings)); + link_ksettings.base.cmd = 0x0000004c; + + link_ksettings.base.link_mode_masks_nwords = + -((s8)(((__ETHTOOL_LINK_MODE_MASK_NBITS) + (32) - 1) / (32))); + + if (copy_to_user(useraddr, &link_ksettings.base, + sizeof(link_ksettings.base))) + return -14; + + return 0; + } + + return 0; +} diff --git a/gcc/testsuite/gcc.dg/analyzer/torture/infoleak-vfio_iommu_type1.c b/gcc/testsuite/gcc.dg/analyzer/torture/infoleak-vfio_iommu_type1.c new file mode 100644 index 00000000000..ddd72d4529c --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/torture/infoleak-vfio_iommu_type1.c @@ -0,0 +1,39 @@ +/* Reduced from infoleak false positive in drivers/vfio/vfio_iommu_type1.c */ + +typedef unsigned int u32; +typedef unsigned long long u64; + +unsigned long +copy_from_user(void *to, const void *from, unsigned long n); + +unsigned long +copy_to_user(void *to, const void *from, unsigned long n); + +struct vfio_iommu_type1_info { + u32 argsz; + u32 flags; + u64 iova_pgsizes; + u32 cap_offset; + /* bytes 20-23 are padding. */ +}; + +int vfio_iommu_type1_get_info(unsigned long arg) +{ + struct vfio_iommu_type1_info info; + unsigned long minsz = 16; + + if (copy_from_user(&info, (void *)arg, 16)) + return -14; + + if (info.argsz < 16) + return -22; + + if (info.argsz >= 20) { + minsz = 20; + info.cap_offset = 0; + } + + /* The padding bytes (20-23) are uninitialized, but can't be written + back, since minsz is either 16 or 20. */ + return copy_to_user((void *)arg, &info, minsz) ? -14 : 0; /* { dg-bogus "exposure" } */ +} From patchwork Sat Nov 13 20:37:31 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 47624 X-Patchwork-Delegate: dmalcolm@redhat.com Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 48A2D3858427 for ; Sat, 13 Nov 2021 20:43:09 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 48A2D3858427 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1636836189; bh=pNEGZ1ZpB51RgE2sE7xsZEUqCYqVo3RJyZvwOybExj8=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=tJ9ez+GOZ1dDdOhL228E8sglj7eFdqlp/dDB/q+HnEwSY8JNUbkND6+WkqruTbEZK TkirOAZGaRUcHw5e7Eq/wQxd5QFtdaYKB/pbpcrCqI2RR5uNgd5tu5MR8XzhZ7n+qE Dq26Vc2HxrzMJFm5YU/jRJ4Jwudj9/SUAMnwS/p8= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 5D61D385800D for ; Sat, 13 Nov 2021 20:37:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 5D61D385800D Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-88-szsjcPW6PxGgX328kuyBnQ-1; Sat, 13 Nov 2021 15:37:44 -0500 X-MC-Unique: szsjcPW6PxGgX328kuyBnQ-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 11DFC875047; Sat, 13 Nov 2021 20:37:44 +0000 (UTC) Received: from t14s.localdomain.com (ovpn-113-54.phx2.redhat.com [10.3.113.54]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9859119D9D; Sat, 13 Nov 2021 20:37:43 +0000 (UTC) To: gcc-patches@gcc.gnu.org, linux-toolchains@vger.kernel.org Subject: [PATCH 5/6] analyzer: use region::untrusted_p in taint detection Date: Sat, 13 Nov 2021 15:37:31 -0500 Message-Id: <20211113203732.2098220-8-dmalcolm@redhat.com> In-Reply-To: <20211113203732.2098220-1-dmalcolm@redhat.com> References: <20211113203732.2098220-1-dmalcolm@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-13.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: David Malcolm via Gcc-patches From: David Malcolm Reply-To: David Malcolm Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" This patch wires up the "untrusted" region logic to the analyzer's taint detection, so that any data copied via a __user pointer (e.g. via a suitably annotated "copy_from_user" decl) is treated as tainted. It includes a series of reproducers for detecting CVE-2011-0521. Unfortunately the analyzer doesn't yet detect the issue until the code has been significantly simplified from its original form: currently only in -5.c and -6.c in the series of tests (see notes in the individual cases). gcc/analyzer/ChangeLog: * sm-taint.cc (taint_state_machine::get_default_state): New, using region::untrusted_p. gcc/testsuite/ChangeLog: * gcc.dg/analyzer/taint-CVE-2011-0521-1-fixed.c: New test. * gcc.dg/analyzer/taint-CVE-2011-0521-1.c: New test. * gcc.dg/analyzer/taint-CVE-2011-0521-2-fixed.c: New test. * gcc.dg/analyzer/taint-CVE-2011-0521-2.c: New test. * gcc.dg/analyzer/taint-CVE-2011-0521-3-fixed.c: New test. * gcc.dg/analyzer/taint-CVE-2011-0521-3.c: New test. * gcc.dg/analyzer/taint-CVE-2011-0521-4.c: New test. * gcc.dg/analyzer/taint-CVE-2011-0521-5.c: New test. * gcc.dg/analyzer/taint-CVE-2011-0521-6.c: New test. * gcc.dg/analyzer/taint-CVE-2011-0521.h: New test. * gcc.dg/analyzer/taint-antipatterns-1.c: New test. * gcc.dg/analyzer/taint-read-through-untrusted-ptr-1.c: New test. Signed-off-by: David Malcolm --- gcc/analyzer/sm-taint.cc | 13 ++ .../analyzer/taint-CVE-2011-0521-1-fixed.c | 113 +++++++++++++++ .../gcc.dg/analyzer/taint-CVE-2011-0521-1.c | 113 +++++++++++++++ .../analyzer/taint-CVE-2011-0521-2-fixed.c | 93 ++++++++++++ .../gcc.dg/analyzer/taint-CVE-2011-0521-2.c | 93 ++++++++++++ .../analyzer/taint-CVE-2011-0521-3-fixed.c | 56 +++++++ .../gcc.dg/analyzer/taint-CVE-2011-0521-3.c | 57 ++++++++ .../gcc.dg/analyzer/taint-CVE-2011-0521-4.c | 40 +++++ .../gcc.dg/analyzer/taint-CVE-2011-0521-5.c | 42 ++++++ .../gcc.dg/analyzer/taint-CVE-2011-0521-6.c | 37 +++++ .../gcc.dg/analyzer/taint-CVE-2011-0521.h | 136 +++++++++++++++++ .../gcc.dg/analyzer/taint-antipatterns-1.c | 137 ++++++++++++++++++ .../taint-read-through-untrusted-ptr-1.c | 37 +++++ 13 files changed, 967 insertions(+) create mode 100644 gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-1-fixed.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-1.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-2-fixed.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-2.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-3-fixed.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-3.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-4.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-5.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-6.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521.h create mode 100644 gcc/testsuite/gcc.dg/analyzer/taint-antipatterns-1.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/taint-read-through-untrusted-ptr-1.c diff --git a/gcc/analyzer/sm-taint.cc b/gcc/analyzer/sm-taint.cc index 0a51a1fe2ea..53ba6f2b30c 100644 --- a/gcc/analyzer/sm-taint.cc +++ b/gcc/analyzer/sm-taint.cc @@ -85,6 +85,19 @@ public: const extrinsic_state &ext_state) const FINAL OVERRIDE; + state_machine::state_t + get_default_state (const svalue *sval) const FINAL OVERRIDE + { + /* Default to "tainted" when reading through a pointer to an untrusted + region. */ + if (const initial_svalue *initial_sval = sval->dyn_cast_initial_svalue ()) + { + if (initial_sval->get_region ()->untrusted_p ()) + return m_tainted; + } + return m_start; + } + bool on_stmt (sm_context *sm_ctxt, const supernode *node, const gimple *stmt) const FINAL OVERRIDE; diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-1-fixed.c b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-1-fixed.c new file mode 100644 index 00000000000..a97896f2266 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-1-fixed.c @@ -0,0 +1,113 @@ +/* See notes in this header. */ +#include "taint-CVE-2011-0521.h" + +// TODO: remove need for this option +/* { dg-additional-options "-fanalyzer-checker=taint" } */ + +/* Adapted from drivers/media/dvb/ttpci/av7110_ca.c */ + +int dvb_ca_ioctl(struct file *file, unsigned int cmd, void *parg) +{ + struct dvb_device *dvbdev = file->private_data; + struct av7110 *av7110 = dvbdev->priv; + unsigned long arg = (unsigned long) parg; + + /* case CA_GET_SLOT_INFO: */ + { + ca_slot_info_t *info=(ca_slot_info_t *)parg; + + if (info->num < 0 || info->num > 1) + return -EINVAL; + av7110->ci_slot[info->num].num = info->num; /* { dg-bogus "attacker-controlled value" } */ + av7110->ci_slot[info->num].type = FW_CI_LL_SUPPORT(av7110->arm_app) ? + CA_CI_LINK : CA_CI; + memcpy(info, &av7110->ci_slot[info->num], sizeof(ca_slot_info_t)); + } + return 0; +} + +static struct dvb_device dvbdev_ca = { + .priv = NULL, + /* [...snip...] */ + .kernel_ioctl = dvb_ca_ioctl, +}; + +/* Adapted from drivers/media/dvb/dvb-core/dvbdev.c */ + +static DEFINE_MUTEX(dvbdev_mutex); + +int dvb_usercopy(struct file *file, + unsigned int cmd, unsigned long arg, + int (*func)(struct file *file, + unsigned int cmd, void *arg)) +{ + char sbuf[128]; + void *mbuf = NULL; + void *parg = NULL; + int err = -1; + + /* Copy arguments into temp kernel buffer */ + switch (_IOC_DIR(cmd)) { + case _IOC_NONE: + /* + * For this command, the pointer is actually an integer + * argument. + */ + parg = (void *) arg; + break; + case _IOC_READ: /* some v4l ioctls are marked wrong ... */ + case _IOC_WRITE: + case (_IOC_WRITE | _IOC_READ): + if (_IOC_SIZE(cmd) <= sizeof(sbuf)) { + parg = sbuf; + } else { + /* too big to allocate from stack */ + mbuf = kmalloc(_IOC_SIZE(cmd),GFP_KERNEL); + if (NULL == mbuf) + return -ENOMEM; + parg = mbuf; + } + + err = -EFAULT; + if (copy_from_user(parg, (void __user *)arg, _IOC_SIZE(cmd))) + goto out; + break; + } + + /* call driver */ + mutex_lock(&dvbdev_mutex); + if ((err = func(file, cmd, parg)) == -ENOIOCTLCMD) + err = -EINVAL; + mutex_unlock(&dvbdev_mutex); + + if (err < 0) + goto out; + + /* Copy results into user buffer */ + switch (_IOC_DIR(cmd)) + { + case _IOC_READ: + case (_IOC_WRITE | _IOC_READ): + if (copy_to_user((void __user *)arg, parg, _IOC_SIZE(cmd))) + err = -EFAULT; + break; + } + +out: + kfree(mbuf); + return err; +} + +long dvb_generic_ioctl(struct file *file, + unsigned int cmd, unsigned long arg) +{ + struct dvb_device *dvbdev = file->private_data; + + if (!dvbdev) + return -ENODEV; + + if (!dvbdev->kernel_ioctl) + return -EINVAL; + + return dvb_usercopy(file, cmd, arg, dvbdev->kernel_ioctl); +} diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-1.c b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-1.c new file mode 100644 index 00000000000..1279f40d948 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-1.c @@ -0,0 +1,113 @@ +/* See notes in this header. */ +#include "taint-CVE-2011-0521.h" + +// TODO: remove need for this option +/* { dg-additional-options "-fanalyzer-checker=taint" } */ + +/* Adapted from drivers/media/dvb/ttpci/av7110_ca.c */ + +int dvb_ca_ioctl(struct file *file, unsigned int cmd, void *parg) +{ + struct dvb_device *dvbdev = file->private_data; + struct av7110 *av7110 = dvbdev->priv; + unsigned long arg = (unsigned long) parg; + + /* case CA_GET_SLOT_INFO: */ + { + ca_slot_info_t *info=(ca_slot_info_t *)parg; + + if (info->num > 1) + return -EINVAL; + av7110->ci_slot[info->num].num = info->num; /* { dg-warning "attacker-controlled value" "" { xfail *-*-* } } */ + av7110->ci_slot[info->num].type = FW_CI_LL_SUPPORT(av7110->arm_app) ? + CA_CI_LINK : CA_CI; + memcpy(info, &av7110->ci_slot[info->num], sizeof(ca_slot_info_t)); + } + return 0; +} + +static struct dvb_device dvbdev_ca = { + .priv = NULL, + /* [...snip...] */ + .kernel_ioctl = dvb_ca_ioctl, +}; + +/* Adapted from drivers/media/dvb/dvb-core/dvbdev.c */ + +static DEFINE_MUTEX(dvbdev_mutex); + +int dvb_usercopy(struct file *file, + unsigned int cmd, unsigned long arg, + int (*func)(struct file *file, + unsigned int cmd, void *arg)) +{ + char sbuf[128]; + void *mbuf = NULL; + void *parg = NULL; + int err = -1; + + /* Copy arguments into temp kernel buffer */ + switch (_IOC_DIR(cmd)) { + case _IOC_NONE: + /* + * For this command, the pointer is actually an integer + * argument. + */ + parg = (void *) arg; + break; + case _IOC_READ: /* some v4l ioctls are marked wrong ... */ + case _IOC_WRITE: + case (_IOC_WRITE | _IOC_READ): + if (_IOC_SIZE(cmd) <= sizeof(sbuf)) { + parg = sbuf; + } else { + /* too big to allocate from stack */ + mbuf = kmalloc(_IOC_SIZE(cmd),GFP_KERNEL); + if (NULL == mbuf) + return -ENOMEM; + parg = mbuf; + } + + err = -EFAULT; + if (copy_from_user(parg, (void __user *)arg, _IOC_SIZE(cmd))) + goto out; + break; + } + + /* call driver */ + mutex_lock(&dvbdev_mutex); + if ((err = func(file, cmd, parg)) == -ENOIOCTLCMD) + err = -EINVAL; + mutex_unlock(&dvbdev_mutex); + + if (err < 0) + goto out; + + /* Copy results into user buffer */ + switch (_IOC_DIR(cmd)) + { + case _IOC_READ: + case (_IOC_WRITE | _IOC_READ): + if (copy_to_user((void __user *)arg, parg, _IOC_SIZE(cmd))) + err = -EFAULT; + break; + } + +out: + kfree(mbuf); + return err; +} + +long dvb_generic_ioctl(struct file *file, + unsigned int cmd, unsigned long arg) +{ + struct dvb_device *dvbdev = file->private_data; + + if (!dvbdev) + return -ENODEV; + + if (!dvbdev->kernel_ioctl) + return -EINVAL; + + return dvb_usercopy(file, cmd, arg, dvbdev->kernel_ioctl); +} diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-2-fixed.c b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-2-fixed.c new file mode 100644 index 00000000000..2b06bde4063 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-2-fixed.c @@ -0,0 +1,93 @@ +/* See notes in this header. */ +#include "taint-CVE-2011-0521.h" + +// TODO: remove need for this option +/* { dg-additional-options "-fanalyzer-checker=taint" } */ + +/* Adapted from drivers/media/dvb/ttpci/av7110_ca.c */ + +int dvb_ca_ioctl(struct file *file, unsigned int cmd, void *parg) +{ + struct dvb_device *dvbdev = file->private_data; + struct av7110 *av7110 = dvbdev->priv; + unsigned long arg = (unsigned long) parg; + + /* case CA_GET_SLOT_INFO: */ + { + ca_slot_info_t *info=(ca_slot_info_t *)parg; + + if (info->num < 0 || info->num > 1) + return -EINVAL; + av7110->ci_slot[info->num].num = info->num; /* { dg-bogus "attacker-controlled value" } */ + av7110->ci_slot[info->num].type = FW_CI_LL_SUPPORT(av7110->arm_app) ? + CA_CI_LINK : CA_CI; + memcpy(info, &av7110->ci_slot[info->num], sizeof(ca_slot_info_t)); + } + return 0; +} + +/* Adapted from drivers/media/dvb/dvb-core/dvbdev.c + Somewhat simplified: rather than pass in a callback that can + be dvb_ca_ioctl, call dvb_ca_ioctl directly. */ + +static DEFINE_MUTEX(dvbdev_mutex); + +int dvb_usercopy(struct file *file, + unsigned int cmd, unsigned long arg) +{ + char sbuf[128]; + void *mbuf = NULL; + void *parg = NULL; + int err = -1; + + /* Copy arguments into temp kernel buffer */ + switch (_IOC_DIR(cmd)) { + case _IOC_NONE: + /* + * For this command, the pointer is actually an integer + * argument. + */ + parg = (void *) arg; + break; + case _IOC_READ: /* some v4l ioctls are marked wrong ... */ + case _IOC_WRITE: + case (_IOC_WRITE | _IOC_READ): + if (_IOC_SIZE(cmd) <= sizeof(sbuf)) { + parg = sbuf; + } else { + /* too big to allocate from stack */ + mbuf = kmalloc(_IOC_SIZE(cmd),GFP_KERNEL); + if (NULL == mbuf) + return -ENOMEM; + parg = mbuf; + } + + err = -EFAULT; + if (copy_from_user(parg, (void __user *)arg, _IOC_SIZE(cmd))) + goto out; + break; + } + + /* call driver */ + mutex_lock(&dvbdev_mutex); + if ((err = dvb_ca_ioctl(file, cmd, parg)) == -ENOIOCTLCMD) + err = -EINVAL; + mutex_unlock(&dvbdev_mutex); + + if (err < 0) + goto out; + + /* Copy results into user buffer */ + switch (_IOC_DIR(cmd)) + { + case _IOC_READ: + case (_IOC_WRITE | _IOC_READ): + if (copy_to_user((void __user *)arg, parg, _IOC_SIZE(cmd))) + err = -EFAULT; + break; + } + +out: + kfree(mbuf); + return err; +} diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-2.c b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-2.c new file mode 100644 index 00000000000..c1bf748ae15 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-2.c @@ -0,0 +1,93 @@ +/* See notes in this header. */ +#include "taint-CVE-2011-0521.h" + +// TODO: remove need for this option +/* { dg-additional-options "-fanalyzer-checker=taint" } */ + +/* Adapted from drivers/media/dvb/ttpci/av7110_ca.c */ + +int dvb_ca_ioctl(struct file *file, unsigned int cmd, void *parg) +{ + struct dvb_device *dvbdev = file->private_data; + struct av7110 *av7110 = dvbdev->priv; + unsigned long arg = (unsigned long) parg; + + /* case CA_GET_SLOT_INFO: */ + { + ca_slot_info_t *info=(ca_slot_info_t *)parg; + + if (info->num > 1) + return -EINVAL; + av7110->ci_slot[info->num].num = info->num; /* { dg-warning "attacker-controlled value" "" { xfail *-*-* } } */ + av7110->ci_slot[info->num].type = FW_CI_LL_SUPPORT(av7110->arm_app) ? + CA_CI_LINK : CA_CI; + memcpy(info, &av7110->ci_slot[info->num], sizeof(ca_slot_info_t)); + } + return 0; +} + +/* Adapted from drivers/media/dvb/dvb-core/dvbdev.c + Somewhat simplified: rather than pass in a callback that can + be dvb_ca_ioctl, call dvb_ca_ioctl directly. */ + +static DEFINE_MUTEX(dvbdev_mutex); + +int dvb_usercopy(struct file *file, + unsigned int cmd, unsigned long arg) +{ + char sbuf[128]; + void *mbuf = NULL; + void *parg = NULL; + int err = -1; + + /* Copy arguments into temp kernel buffer */ + switch (_IOC_DIR(cmd)) { + case _IOC_NONE: + /* + * For this command, the pointer is actually an integer + * argument. + */ + parg = (void *) arg; + break; + case _IOC_READ: /* some v4l ioctls are marked wrong ... */ + case _IOC_WRITE: + case (_IOC_WRITE | _IOC_READ): + if (_IOC_SIZE(cmd) <= sizeof(sbuf)) { + parg = sbuf; + } else { + /* too big to allocate from stack */ + mbuf = kmalloc(_IOC_SIZE(cmd),GFP_KERNEL); + if (NULL == mbuf) + return -ENOMEM; + parg = mbuf; + } + + err = -EFAULT; + if (copy_from_user(parg, (void __user *)arg, _IOC_SIZE(cmd))) + goto out; + break; + } + + /* call driver */ + mutex_lock(&dvbdev_mutex); + if ((err = dvb_ca_ioctl(file, cmd, parg)) == -ENOIOCTLCMD) + err = -EINVAL; + mutex_unlock(&dvbdev_mutex); + + if (err < 0) + goto out; + + /* Copy results into user buffer */ + switch (_IOC_DIR(cmd)) + { + case _IOC_READ: + case (_IOC_WRITE | _IOC_READ): + if (copy_to_user((void __user *)arg, parg, _IOC_SIZE(cmd))) + err = -EFAULT; + break; + } + +out: + kfree(mbuf); + return err; +} diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-3-fixed.c b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-3-fixed.c new file mode 100644 index 00000000000..0147759f4df --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-3-fixed.c @@ -0,0 +1,56 @@ +/* See notes in this header. */ +#include "taint-CVE-2011-0521.h" + +// TODO: remove need for this option +/* { dg-additional-options "-fanalyzer-checker=taint" } */ + +/* Adapted from drivers/media/dvb/ttpci/av7110_ca.c */ + +int dvb_ca_ioctl(struct file *file, unsigned int cmd, void *parg) +{ + struct dvb_device *dvbdev = file->private_data; + struct av7110 *av7110 = dvbdev->priv; + unsigned long arg = (unsigned long) parg; + + /* case CA_GET_SLOT_INFO: */ + { + ca_slot_info_t *info=(ca_slot_info_t *)parg; + + if (info->num < 0 || info->num > 1) + return -EINVAL; + av7110->ci_slot[info->num].num = info->num; /* { dg-bogus "attacker-controlled value" } */ + av7110->ci_slot[info->num].type = FW_CI_LL_SUPPORT(av7110->arm_app) ? + CA_CI_LINK : CA_CI; + memcpy(info, &av7110->ci_slot[info->num], sizeof(ca_slot_info_t)); + } + return 0; +} + +/* Adapted from drivers/media/dvb/dvb-core/dvbdev.c + Further simplified from -2; always use an on-stack buffer. */ + +static DEFINE_MUTEX(dvbdev_mutex); + +int dvb_usercopy(struct file *file, + unsigned int cmd, unsigned long arg) +{ + char sbuf[128]; + void *parg = sbuf; + int err = -EFAULT; + if (copy_from_user(parg, (void __user *)arg, sizeof(sbuf))) + goto out; + + mutex_lock(&dvbdev_mutex); + if ((err = dvb_ca_ioctl(file, cmd, parg)) == -ENOIOCTLCMD) + err = -EINVAL; + mutex_unlock(&dvbdev_mutex); + + if (err < 0) + goto out; + + if (copy_to_user((void __user *)arg, parg, sizeof(sbuf))) + err = -EFAULT; + +out: + return err; +} diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-3.c b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-3.c new file mode 100644 index 00000000000..c53071afbab --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-3.c @@ -0,0 +1,57 @@ +/* See notes in this header. */ +#include "taint-CVE-2011-0521.h" + +// TODO: remove need for this option +/* { dg-additional-options "-fanalyzer-checker=taint" } */ + +/* Adapted from drivers/media/dvb/ttpci/av7110_ca.c */ + +int dvb_ca_ioctl(struct file *file, unsigned int cmd, void *parg) +{ + struct dvb_device *dvbdev = file->private_data; + struct av7110 *av7110 = dvbdev->priv; + unsigned long arg = (unsigned long) parg; + + /* case CA_GET_SLOT_INFO: */ + { + ca_slot_info_t *info=(ca_slot_info_t *)parg; + + if (info->num > 1) + return -EINVAL; + av7110->ci_slot[info->num].num = info->num; /* { dg-warning "attacker-controlled value" "" { xfail *-*-* } } */ + // TODO(xfail) + av7110->ci_slot[info->num].type = FW_CI_LL_SUPPORT(av7110->arm_app) ? + CA_CI_LINK : CA_CI; + memcpy(info, &av7110->ci_slot[info->num], sizeof(ca_slot_info_t)); + } + return 0; +} + +/* Adapted from drivers/media/dvb/dvb-core/dvbdev.c + Further simplified from -2; always use an on-stack buffer. */ + +static DEFINE_MUTEX(dvbdev_mutex); + +int dvb_usercopy(struct file *file, + unsigned int cmd, unsigned long arg) +{ + char sbuf[128]; + void *parg = sbuf; + int err = -EFAULT; + if (copy_from_user(parg, (void __user *)arg, sizeof(sbuf))) + goto out; + + mutex_lock(&dvbdev_mutex); + if ((err = dvb_ca_ioctl(file, cmd, parg)) == -ENOIOCTLCMD) + err = -EINVAL; + mutex_unlock(&dvbdev_mutex); + + if (err < 0) + goto out; + + if (copy_to_user((void __user *)arg, parg, sizeof(sbuf))) + err = -EFAULT; + +out: + return err; +} diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-4.c b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-4.c new file mode 100644 index 00000000000..eab95929cd6 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-4.c @@ -0,0 +1,40 @@ +/* See notes in this header. */ +#include "taint-CVE-2011-0521.h" + +// TODO: remove need for this option +/* { dg-additional-options "-fanalyzer-checker=taint" } */ + +/* Adapted from dvb_ca_ioctl in drivers/media/dvb/ttpci/av7110_ca.c and + dvb_usercopy in drivers/media/dvb/dvb-core/dvbdev.c + + Further simplified from -3; merge into a single function; drop the mutex, + remove control flow. */ + +int test_1(struct file *file, unsigned int cmd, unsigned long arg) +{ + char sbuf[128]; + void *parg = sbuf; + + copy_from_user(parg, (void __user *)arg, sizeof(sbuf)); + + { + struct dvb_device *dvbdev = file->private_data; + struct av7110 *av7110 = dvbdev->priv; + unsigned long arg = (unsigned long) parg; + + /* case CA_GET_SLOT_INFO: */ + ca_slot_info_t *info=(ca_slot_info_t *)parg; + + if (info->num > 1) + return -EINVAL; + av7110->ci_slot[info->num].num = info->num; /* { dg-warning "attacker-controlled value" "" { xfail *-*-* } } */ + // TODO(xfail) + av7110->ci_slot[info->num].type = FW_CI_LL_SUPPORT(av7110->arm_app) ? + CA_CI_LINK : CA_CI; + memcpy(info, &av7110->ci_slot[info->num], sizeof(ca_slot_info_t)); + } + + copy_to_user((void __user *)arg, parg, sizeof(sbuf)); + + return 0; +} diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-5.c b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-5.c new file mode 100644 index 00000000000..9cf465204cc --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-5.c @@ -0,0 +1,42 @@ +/* See notes in this header. */ +#include "taint-CVE-2011-0521.h" + +// TODO: remove need for this option +/* { dg-additional-options "-fanalyzer-checker=taint" } */ + +/* Adapted from dvb_ca_ioctl in drivers/media/dvb/ttpci/av7110_ca.c and + dvb_usercopy in drivers/media/dvb/dvb-core/dvbdev.c + + Further simplified from -4; avoid parg and the cast to char[128]. */ + +int test_1(struct file *file, unsigned int cmd, unsigned long arg) +{ + ca_slot_info_t sbuf; + + if (copy_from_user(&sbuf, (void __user *)arg, sizeof(sbuf)) != 0) + return -1; + + { + struct dvb_device *dvbdev = file->private_data; + struct av7110 *av7110 = dvbdev->priv; + + /* case CA_GET_SLOT_INFO: */ + ca_slot_info_t *info= &sbuf; + + __analyzer_dump_state ("taint", info->num); /* { dg-warning "tainted" } */ + + if (info->num > 1) + return -EINVAL; + + __analyzer_dump_state ("taint", info->num); /* { dg-warning "has_ub" } */ + + av7110->ci_slot[info->num].num = info->num; /* { dg-warning "use of attacker-controlled value '\\*info\\.num' in array lookup without checking for negative" } */ + av7110->ci_slot[info->num].type = FW_CI_LL_SUPPORT(av7110->arm_app) ? + CA_CI_LINK : CA_CI; + memcpy(info, &av7110->ci_slot[info->num], sizeof(ca_slot_info_t)); + } + + copy_to_user((void __user *)arg, &sbuf, sizeof(sbuf)); + + return 0; +} diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-6.c b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-6.c new file mode 100644 index 00000000000..35a16af2316 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521-6.c @@ -0,0 +1,37 @@ +/* See notes in this header. */ +#include "taint-CVE-2011-0521.h" + +// TODO: remove need for this option +/* { dg-additional-options "-fanalyzer-checker=taint" } */ + +/* Adapted from dvb_ca_ioctl in drivers/media/dvb/ttpci/av7110_ca.c and + dvb_usercopy in drivers/media/dvb/dvb-core/dvbdev.c + + Further simplified from -5; remove all control flow. */ + +int test_1(struct file *file, unsigned int cmd, unsigned long arg) +{ + ca_slot_info_t sbuf; + + if (copy_from_user(&sbuf, (void __user *)arg, sizeof(sbuf)) != 0) + return -1; + + { + struct dvb_device *dvbdev = file->private_data; + struct av7110 *av7110 = dvbdev->priv; + + /* case CA_GET_SLOT_INFO: */ + ca_slot_info_t *info= &sbuf; + + __analyzer_dump_state ("taint", info->num); /* { dg-warning "tainted" } */ + + av7110->ci_slot[info->num].num = info->num; /* { dg-warning "use of attacker-controlled value '\\*info\\.num' in array lookup without bounds checking" } */ + av7110->ci_slot[info->num].type = FW_CI_LL_SUPPORT(av7110->arm_app) ? + CA_CI_LINK : CA_CI; + memcpy(info, &av7110->ci_slot[info->num], sizeof(ca_slot_info_t)); + } + + copy_to_user((void __user *)arg, &sbuf, sizeof(sbuf)); + + return 0; +} diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521.h b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521.h new file mode 100644 index 00000000000..0d79f9f9e08 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-0521.h @@ -0,0 +1,136 @@ +/* Shared header for the various taint-CVE-2011-0521-*.c tests. + These are a series of successively simpler reductions of the reproducer. + Ideally the analyzer would detect the issue in all of the testcases, + but currently requires some simplification of the code to do so. + + "The dvb_ca_ioctl function in drivers/media/dvb/ttpci/av7110_ca.c in the + Linux kernel before 2.6.38-rc2 does not check the sign of a certain integer + field, which allows local users to cause a denial of service (memory + corruption) or possibly have unspecified other impact via a negative value." + + Adapted from Linux 2.6.38, which is under the GPLv2. + + Fixed in e.g. cb26a24ee9706473f31d34cc259f4dcf45cd0644 on linux-2.6.38.y */ + +#include +#include "test-uaccess.h" +#include "analyzer-decls.h" + +typedef unsigned int u32; + +/* Adapted from include/linux/compiler.h */ + +#define __force + +/* Adapted from include/asm-generic/errno-base.h */ + +#define ENOMEM 12 /* Out of memory */ +#define EFAULT 14 /* Bad address */ +#define ENODEV 19 /* No such device */ +#define EINVAL 22 /* Invalid argument */ + +/* Adapted from include/linux/errno.h */ + +#define ENOIOCTLCMD 515 /* No ioctl command */ + +/* Adapted from include/linux/fs.h */ + +struct file { + /* [...snip...] */ + void *private_data; + /* [...snip...] */ +}; + +/* Adapted from drivers/media/dvb/dvb-core/dvbdev.h */ + +struct dvb_device { + /* [...snip...] */ + int (*kernel_ioctl)(struct file *file, unsigned int cmd, void *arg); + + void *priv; +}; + + +/* Adapted from include/linux/dvb/ca.h */ + +typedef struct ca_slot_info { + int num; /* slot number */ + + int type; /* CA interface this slot supports */ +#define CA_CI 1 /* CI high level interface */ +#define CA_CI_LINK 2 /* CI link layer level interface */ + /* [...snip...] */ +} ca_slot_info_t; + + +/* Adapted from drivers/media/dvb/ttpci/av7110.h */ + +struct av7110 { + /* [...snip...] */ + ca_slot_info_t ci_slot[2]; + /* [...snip...] */ + u32 arm_app; + /* [...snip...] */ +}; + +/* Adapted from drivers/media/dvb/ttpci/av7110_hw.h */ + +#define FW_CI_LL_SUPPORT(arm_app) ((arm_app) & 0x80000000) + +/* Adapted from include/asm-generic/ioctl.h */ + +#define _IOC_NRBITS 8 +#define _IOC_TYPEBITS 8 + +#define _IOC_SIZEBITS 14 +#define _IOC_DIRBITS 2 + +#define _IOC_SIZEMASK ((1 << _IOC_SIZEBITS)-1) +#define _IOC_DIRMASK ((1 << _IOC_DIRBITS)-1) +#define _IOC_NRSHIFT 0 +#define _IOC_TYPESHIFT (_IOC_NRSHIFT+_IOC_NRBITS) +#define _IOC_SIZESHIFT (_IOC_TYPESHIFT+_IOC_TYPEBITS) +#define _IOC_DIRSHIFT (_IOC_SIZESHIFT+_IOC_SIZEBITS) + +#define _IOC_NONE 0U +#define _IOC_WRITE 1U +#define _IOC_READ 2U + +#define _IOC_DIR(nr) (((nr) >> _IOC_DIRSHIFT) & _IOC_DIRMASK) +#define _IOC_SIZE(nr) (((nr) >> _IOC_SIZESHIFT) & _IOC_SIZEMASK) + +/* Adapted from include/linux/mutex.h */ + +struct mutex { + /* [...snip...] */ +}; + +#define __MUTEX_INITIALIZER(lockname) \ + { /* [...snip...] */ } + +#define DEFINE_MUTEX(mutexname) \ + struct mutex mutexname = __MUTEX_INITIALIZER(mutexname) + +extern void mutex_lock(struct mutex *lock); +extern void mutex_unlock(struct mutex *lock); + +/* Adapted from include/linux/types.h */ + +#define __bitwise__ +typedef unsigned __bitwise__ gfp_t; + +/* Adapted from include/linux/gfp.h */ + +#define ___GFP_WAIT 0x10u +#define ___GFP_IO 0x40u +#define ___GFP_FS 0x80u +#define __GFP_WAIT ((__force gfp_t)___GFP_WAIT) +#define __GFP_IO ((__force gfp_t)___GFP_IO) +#define __GFP_FS ((__force gfp_t)___GFP_FS) +#define GFP_KERNEL (__GFP_WAIT | __GFP_IO | __GFP_FS) + +/* Adapted from include/linux/slab.h */ + +void kfree(const void *); +void *kmalloc(size_t size, gfp_t flags) + __attribute__((malloc (kfree))); diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-antipatterns-1.c b/gcc/testsuite/gcc.dg/analyzer/taint-antipatterns-1.c new file mode 100644 index 00000000000..5e81410a847 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/taint-antipatterns-1.c @@ -0,0 +1,137 @@ +// TODO: remove need for this: +/* { dg-additional-options "-fanalyzer-checker=taint" } */ + +#include "test-uaccess.h" + +/* Adapted and simplified decls from linux kernel headers. */ + +typedef unsigned char u8; +typedef unsigned __INT16_TYPE__ u16; +typedef unsigned __INT32_TYPE__ u32; +typedef signed __INT32_TYPE__ s32; +typedef __SIZE_TYPE__ size_t; + +#define EFAULT 14 + +typedef unsigned int gfp_t; +#define GFP_KERNEL 0 + +void kfree(const void *); +void *kmalloc(size_t size, gfp_t flags) + __attribute__((malloc (kfree))); + +/* Adapted from antipatterns.ko:taint.c (GPL-v2.0). */ + +struct cmd_1 +{ + u32 idx; + u32 val; +}; + +static u32 arr[16]; + +int taint_array_access(void __user *src) +{ + struct cmd_1 cmd; + if (copy_from_user(&cmd, src, sizeof(cmd))) + return -EFAULT; + /* + * cmd.idx is an unsanitized value from user-space, hence + * this is an arbitrary kernel memory access. + */ + arr[cmd.idx] = cmd.val; /* { dg-warning "use of attacker-controlled value 'cmd.idx' in array lookup without upper-bounds checking" } */ + return 0; +} + +struct cmd_2 +{ + s32 idx; + u32 val; +}; + +int taint_signed_array_access(void __user *src) +{ + struct cmd_2 cmd; + if (copy_from_user(&cmd, src, sizeof(cmd))) + return -EFAULT; + if (cmd.idx >= 16) + return -EFAULT; + + /* + * cmd.idx hasn't been checked for being negative, hence + * this is an arbitrary kernel memory access. + */ + arr[cmd.idx] = cmd.val; /* { dg-warning "use of attacker-controlled value 'cmd.idx' in array lookup without checking for negative" } */ + return 0; +} + +struct cmd_s32_binop +{ + s32 a; + s32 b; + s32 result; +}; + +int taint_divide_by_zero_direct(void __user *uptr) +{ + struct cmd_s32_binop cmd; + if (copy_from_user(&cmd, uptr, sizeof(cmd))) + return -EFAULT; + + /* cmd.b is attacker-controlled and could be zero */ + cmd.result = cmd.a / cmd.b; /* { dg-warning "use of attacker-controlled value 'cmd.b' as divisor without checking for zero" } */ + + if (copy_to_user (uptr, &cmd, sizeof(cmd))) + return -EFAULT; + return 0; +} + +int taint_divide_by_zero_compound(void __user *uptr) +{ + struct cmd_s32_binop cmd; + if (copy_from_user(&cmd, uptr, sizeof(cmd))) + return -EFAULT; + + /* + * cmd.b is attacker-controlled and could be -1, hence + * the divisor could be zero + */ + cmd.result = cmd.a / (cmd.b + 1); /* { dg-warning "use of attacker-controlled value 'cmd.b \\+ 1' as divisor without checking for zero" } */ + + if (copy_to_user (uptr, &cmd, sizeof(cmd))) + return -EFAULT; + return 0; +} + +int taint_mod_by_zero_direct(void __user *uptr) +{ + struct cmd_s32_binop cmd; + if (copy_from_user(&cmd, uptr, sizeof(cmd))) + return -EFAULT; + + /* cmd.b is attacker-controlled and could be zero */ + cmd.result = cmd.a % cmd.b; /* { dg-warning "use of attacker-controlled value 'cmd.b' as divisor without checking for zero" } */ + + if (copy_to_user (uptr, &cmd, sizeof(cmd))) + return -EFAULT; + return 0; +} + +int taint_mod_by_zero_compound(void __user *uptr) +{ + struct cmd_s32_binop cmd; + if (copy_from_user(&cmd, uptr, sizeof(cmd))) + return -EFAULT; + + /* + * cmd.b is attacker-controlled and could be -1, hence + * the divisor could be zero + */ + cmd.result = cmd.a % (cmd.b + 1); /* { dg-warning "use of attacker-controlled value 'cmd.b \\+ 1' as divisor without checking for zero" } */ + + if (copy_to_user (uptr, &cmd, sizeof(cmd))) + return -EFAULT; + return 0; +} + +/* TODO: etc. */ diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-read-through-untrusted-ptr-1.c b/gcc/testsuite/gcc.dg/analyzer/taint-read-through-untrusted-ptr-1.c new file mode 100644 index 00000000000..cd2911683e6 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/taint-read-through-untrusted-ptr-1.c @@ -0,0 +1,37 @@ +// TODO: remove need for this: +/* { dg-additional-options "-fanalyzer-checker=taint" } */ + +#include "test-uaccess.h" + +typedef unsigned __INT32_TYPE__ u32; + +struct cmd_1 +{ + u32 idx; + u32 val; +}; + +u32 arr[16]; + +int taint_array_access_1 (struct cmd_1 __user *src) +{ + /* + * src->idx is an unsanitized value from user-space, hence + * this is an arbitrary kernel memory access. + */ + arr[src->idx] = src->val; /* { dg-warning "use of attacker-controlled value '\\*src.idx' in array lookup without upper-bounds checking" } */ + return 0; +} + +int taint_array_access_2 (struct cmd_1 __user *src) +{ + struct cmd_1 cmd; + cmd = *src; + + /* + * cmd.idx is an unsanitized value from user-space, hence + * this is an arbitrary kernel memory access. + */ + arr[cmd.idx] = cmd.val; /* { dg-warning "use of attacker-controlled value '\\*src.idx' in array lookup without upper-bounds checking" } */ + return 0; +} From patchwork Sat Nov 13 20:37:32 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 47626 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 94DFA3857C5F for ; Sat, 13 Nov 2021 20:45:21 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 94DFA3857C5F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1636836321; bh=K3cRfOBW6SNFHgwzJNxpxjtoDnYhoVJS3Ls9leLI4+s=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=RHx1PwYgH/rWeA1+bZAcgkx9GaoQQtqMJSNOSRkoS/aqvoNq59siGWnxW8S26FgrT U+wxZ0hKMxO3veMSzJJDXa9VZnkSUXJHj6LtiVtSoLQA5gAX333DMYa/3muK7e5Iyc s/VDANlz5LWyCTwLXGcRxzcua+XkCsMAkGTa0FQ8= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id B00A73858410 for ; Sat, 13 Nov 2021 20:37:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B00A73858410 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-19-c_twdSMROaWlFKdtvRTUtg-1; Sat, 13 Nov 2021 15:37:45 -0500 X-MC-Unique: c_twdSMROaWlFKdtvRTUtg-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A7F3E180831B; Sat, 13 Nov 2021 20:37:44 +0000 (UTC) Received: from t14s.localdomain.com (ovpn-113-54.phx2.redhat.com [10.3.113.54]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3A5F019D9D; Sat, 13 Nov 2021 20:37:44 +0000 (UTC) To: gcc-patches@gcc.gnu.org, linux-toolchains@vger.kernel.org Subject: [PATCH 6/6] Add __attribute__ ((tainted)) Date: Sat, 13 Nov 2021 15:37:32 -0500 Message-Id: <20211113203732.2098220-9-dmalcolm@redhat.com> In-Reply-To: <20211113203732.2098220-1-dmalcolm@redhat.com> References: <20211113203732.2098220-1-dmalcolm@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-13.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_FILL_THIS_FORM_SHORT autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: David Malcolm via Gcc-patches From: David Malcolm Reply-To: David Malcolm Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" This patch adds a new __attribute__ ((tainted)) to the C/C++ frontends. It can be used on function decls: the analyzer will treat as tainted all parameters to the function and all buffers pointed to by parameters to the function. Adding this in one place to the Linux kernel's __SYSCALL_DEFINEx macro allows the analyzer to treat all syscalls as having tainted inputs. This gives additional testing beyond e.g. __user pointers added by earlier patches - an example of the use of this can be seen in CVE-2011-2210, where given: SYSCALL_DEFINE5(osf_getsysinfo, unsigned long, op, void __user *, buffer, unsigned long, nbytes, int __user *, start, void __user *, arg) the analyzer will treat the nbytes param as under attacker control, and can complain accordingly: taint-CVE-2011-2210-1.c: In function ‘sys_osf_getsysinfo’: taint-CVE-2011-2210-1.c:69:21: warning: use of attacker-controlled value ‘nbytes’ as size without upper-bounds checking [CWE-129] [-Wanalyzer-tainted-size] 69 | if (copy_to_user(buffer, hwrpb, nbytes) != 0) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Additionally, the patch allows the attribute to be used on field decls: specifically function pointers. Any function used as an initializer for such a field gets treated as tainted. An example can be seen in CVE-2020-13143, where adding __attribute__((tainted)) to the "store" callback of configfs_attribute: struct configfs_attribute { /* [...snip...] */ ssize_t (*store)(struct config_item *, const char *, size_t) __attribute__((tainted)); /* [...snip...] */ }; allows the analyzer to see: CONFIGFS_ATTR(gadget_dev_desc_, UDC); and treat gadget_dev_desc_UDC_store as tainted, so that it complains: taint-CVE-2020-13143-1.c: In function ‘gadget_dev_desc_UDC_store’: taint-CVE-2020-13143-1.c:33:17: warning: use of attacker-controlled value ‘len + 18446744073709551615’ as offset without upper-bounds checking [CWE-823] [-Wanalyzer-tainted-offset] 33 | if (name[len - 1] == '\n') | ~~~~^~~~~~~~~ Similarly, the attribute could be used on the ioctl callback field, USB device callbacks, network-handling callbacks etc. This potentially gives a lot of test coverage with relatively little code annotation, and without necessarily needing link-time analysis (which -fanalyzer can only do at present on trivial examples). I believe this is the first time we've had an attribute on a field. If that's an issue, I could prepare a version of the patch that merely allowed it on functions themselves. As before this currently still needs -fanalyzer-checker=taint (in addition to -fanalyzer). gcc/analyzer/ChangeLog: * engine.cc: Include "stringpool.h", "attribs.h", and "tree-dfa.h". (mark_params_as_tainted): New. (class tainted_function_custom_event): New. (class tainted_function_info): New. (exploded_graph::add_function_entry): Handle functions with "tainted" attribute. (class tainted_field_custom_event): New. (class tainted_callback_custom_event): New. (class tainted_call_info): New. (add_tainted_callback): New. (add_any_callbacks): New. (exploded_graph::build_initial_worklist): Find callbacks that are reachable from global initializers, calling add_any_callbacks on them. gcc/c-family/ChangeLog: * c-attribs.c (c_common_attribute_table): Add "tainted". (handle_tainted_attribute): New. gcc/ChangeLog: * doc/extend.texi (Function Attributes): Note that "tainted" can be used on field decls. (Common Function Attributes): Add entry on "tainted" attribute. gcc/testsuite/ChangeLog: * gcc.dg/analyzer/attr-tainted-1.c: New test. * gcc.dg/analyzer/attr-tainted-misuses.c: New test. * gcc.dg/analyzer/taint-CVE-2011-2210-1.c: New test. * gcc.dg/analyzer/taint-CVE-2020-13143-1.c: New test. * gcc.dg/analyzer/taint-CVE-2020-13143-2.c: New test. * gcc.dg/analyzer/taint-CVE-2020-13143.h: New test. * gcc.dg/analyzer/taint-alloc-3.c: New test. * gcc.dg/analyzer/taint-alloc-4.c: New test. Signed-off-by: David Malcolm --- gcc/analyzer/engine.cc | 317 +++++++++++++++++- gcc/c-family/c-attribs.c | 36 ++ gcc/doc/extend.texi | 22 +- .../gcc.dg/analyzer/attr-tainted-1.c | 88 +++++ .../gcc.dg/analyzer/attr-tainted-misuses.c | 6 + .../gcc.dg/analyzer/taint-CVE-2011-2210-1.c | 93 +++++ .../gcc.dg/analyzer/taint-CVE-2020-13143-1.c | 38 +++ .../gcc.dg/analyzer/taint-CVE-2020-13143-2.c | 32 ++ .../gcc.dg/analyzer/taint-CVE-2020-13143.h | 91 +++++ gcc/testsuite/gcc.dg/analyzer/taint-alloc-3.c | 21 ++ gcc/testsuite/gcc.dg/analyzer/taint-alloc-4.c | 31 ++ 11 files changed, 772 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/attr-tainted-1.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/attr-tainted-misuses.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-2210-1.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/taint-CVE-2020-13143-1.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/taint-CVE-2020-13143-2.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/taint-CVE-2020-13143.h create mode 100644 gcc/testsuite/gcc.dg/analyzer/taint-alloc-3.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/taint-alloc-4.c diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc index 096e219392d..5fab41daf93 100644 --- a/gcc/analyzer/engine.cc +++ b/gcc/analyzer/engine.cc @@ -68,6 +68,9 @@ along with GCC; see the file COPYING3. If not see #include "plugin.h" #include "target.h" #include +#include "stringpool.h" +#include "attribs.h" +#include "tree-dfa.h" /* For an overview, see gcc/doc/analyzer.texi. */ @@ -2276,6 +2279,116 @@ exploded_graph::~exploded_graph () delete (*iter).second; } +/* Subroutine for use when implementing __attribute__((tainted)) + on functions and on function pointer fields in structs. + + Called on STATE representing a call to FNDECL. + Mark all params of FNDECL in STATE as "tainted". Mark the value of all + regions pointed to by params of FNDECL as "tainted". + + Return true if successful; return false if the "taint" state machine + was not found. */ + +static bool +mark_params_as_tainted (program_state *state, tree fndecl, + const extrinsic_state &ext_state) +{ + unsigned taint_sm_idx; + if (!ext_state.get_sm_idx_by_name ("taint", &taint_sm_idx)) + return false; + sm_state_map *smap = state->m_checker_states[taint_sm_idx]; + + const state_machine &sm = ext_state.get_sm (taint_sm_idx); + state_machine::state_t tainted = sm.get_state_by_name ("tainted"); + + region_model_manager *mgr = ext_state.get_model_manager (); + + function *fun = DECL_STRUCT_FUNCTION (fndecl); + gcc_assert (fun); + + for (tree iter_parm = DECL_ARGUMENTS (fndecl); iter_parm; + iter_parm = DECL_CHAIN (iter_parm)) + { + tree param = iter_parm; + if (tree parm_default_ssa = ssa_default_def (fun, iter_parm)) + param = parm_default_ssa; + const region *param_reg = state->m_region_model->get_lvalue (param, NULL); + const svalue *init_sval = mgr->get_or_create_initial_value (param_reg); + smap->set_state (state->m_region_model, init_sval, + tainted, NULL /*origin_new_sval*/, ext_state); + if (POINTER_TYPE_P (TREE_TYPE (param))) + { + const region *pointee_reg = mgr->get_symbolic_region (init_sval); + /* Mark "*param" as tainted. */ + const svalue *init_pointee_sval + = mgr->get_or_create_initial_value (pointee_reg); + smap->set_state (state->m_region_model, init_pointee_sval, + tainted, NULL /*origin_new_sval*/, ext_state); + } + } + + return true; +} + +/* Custom event for use by tainted_function_info when a function + has been marked with __attribute__((tainted)). */ + +class tainted_function_custom_event : public custom_event +{ +public: + tainted_function_custom_event (location_t loc, tree fndecl, int depth) + : custom_event (loc, fndecl, depth), + m_fndecl (fndecl) + { + } + + label_text get_desc (bool can_colorize) const FINAL OVERRIDE + { + return make_label_text + (can_colorize, + "function %qE marked with %<__attribute__((tainted))%>", + m_fndecl); + } + +private: + tree m_fndecl; +}; + +/* Custom exploded_edge info for top-level calls to a function + marked with __attribute__((tainted)). */ + +class tainted_function_info : public custom_edge_info +{ +public: + tainted_function_info (tree fndecl) + : m_fndecl (fndecl) + {} + + void print (pretty_printer *pp) const FINAL OVERRIDE + { + pp_string (pp, "call to tainted function"); + }; + + bool update_model (region_model *, + const exploded_edge *, + region_model_context *) const FINAL OVERRIDE + { + /* No-op. */ + return true; + } + + void add_events_to_path (checker_path *emission_path, + const exploded_edge &) const FINAL OVERRIDE + { + emission_path->add_event + (new tainted_function_custom_event + (DECL_SOURCE_LOCATION (m_fndecl), m_fndecl, 0)); + } + +private: + tree m_fndecl; +}; + /* Ensure that there is an exploded_node representing an external call to FUN, adding it to the worklist if creating it. @@ -2302,14 +2415,25 @@ exploded_graph::add_function_entry (function *fun) program_state state (m_ext_state); state.push_frame (m_ext_state, fun); + custom_edge_info *edge_info = NULL; + + if (lookup_attribute ("tainted", DECL_ATTRIBUTES (fun->decl))) + { + if (mark_params_as_tainted (&state, fun->decl, m_ext_state)) + edge_info = new tainted_function_info (fun->decl); + } + if (!state.m_valid) return NULL; exploded_node *enode = get_or_create_node (point, state, NULL); if (!enode) - return NULL; + { + delete edge_info; + return NULL; + } - add_edge (m_origin, enode, NULL); + add_edge (m_origin, enode, NULL, edge_info); m_functions_with_enodes.add (fun); @@ -2623,6 +2747,184 @@ toplevel_function_p (function *fun, logger *logger) return true; } +/* Custom event for use by tainted_call_info when a callback field has been + marked with __attribute__((tainted)), for labelling the field. */ + +class tainted_field_custom_event : public custom_event +{ +public: + tainted_field_custom_event (tree field) + : custom_event (DECL_SOURCE_LOCATION (field), NULL_TREE, 0), + m_field (field) + { + } + + label_text get_desc (bool can_colorize) const FINAL OVERRIDE + { + return make_label_text (can_colorize, + "field %qE of %qT" + " is marked with %<__attribute__((tainted))%>", + m_field, DECL_CONTEXT (m_field)); + } + +private: + tree m_field; +}; + +/* Custom event for use by tainted_call_info when a callback field has been + marked with __attribute__((tainted)), for labelling the function used + in that callback. */ + +class tainted_callback_custom_event : public custom_event +{ +public: + tainted_callback_custom_event (location_t loc, tree fndecl, int depth, + tree field) + : custom_event (loc, fndecl, depth), + m_field (field) + { + } + + label_text get_desc (bool can_colorize) const FINAL OVERRIDE + { + return make_label_text (can_colorize, + "function %qE used as initializer for field %qE" + " marked with %<__attribute__((tainted))%>", + m_fndecl, m_field); + } + +private: + tree m_field; +}; + +/* Custom edge info for use when adding a function used by a callback field + marked with '__attribute__((tainted))'. */ + +class tainted_call_info : public custom_edge_info +{ +public: + tainted_call_info (tree field, tree fndecl, location_t loc) + : m_field (field), m_fndecl (fndecl), m_loc (loc) + {} + + void print (pretty_printer *pp) const FINAL OVERRIDE + { + pp_string (pp, "call to tainted field"); + }; + + bool update_model (region_model *, + const exploded_edge *, + region_model_context *) const FINAL OVERRIDE + { + /* No-op. */ + return true; + } + + void add_events_to_path (checker_path *emission_path, + const exploded_edge &) const FINAL OVERRIDE + { + /* Show the field in the struct declaration + e.g. "(1) field 'store' is marked with '__attribute__((tainted))'" */ + emission_path->add_event + (new tainted_field_custom_event (m_field)); + + /* Show the callback in the initializer + e.g. + "(2) function 'gadget_dev_desc_UDC_store' used as initializer + for field 'store' marked with '__attribute__((tainted))'". */ + emission_path->add_event + (new tainted_callback_custom_event (m_loc, m_fndecl, 0, m_field)); + } + +private: + tree m_field; + tree m_fndecl; + location_t m_loc; +}; + +/* Given an initializer at LOC for FIELD marked with '__attribute__((tainted))' + initialized with FNDECL, add an entrypoint to FNDECL to EG (and to its + worklist) where the params to FNDECL are marked as tainted. */ + +static void +add_tainted_callback (exploded_graph *eg, tree field, tree fndecl, + location_t loc) +{ + logger *logger = eg->get_logger (); + + LOG_SCOPE (logger); + + if (!gimple_has_body_p (fndecl)) + return; + + const extrinsic_state &ext_state = eg->get_ext_state (); + + function *fun = DECL_STRUCT_FUNCTION (fndecl); + gcc_assert (fun); + + program_point point + = program_point::from_function_entry (eg->get_supergraph (), fun); + program_state state (ext_state); + state.push_frame (ext_state, fun); + + if (!mark_params_as_tainted (&state, fndecl, ext_state)) + return; + + if (!state.m_valid) + return; + + exploded_node *enode = eg->get_or_create_node (point, state, NULL); + if (logger) + { + if (enode) + logger->log ("created EN %i for tainted %qE entrypoint", + enode->m_index, fndecl); + else + { + logger->log ("did not create enode for tainted %qE entrypoint", + fndecl); + return; + } + } + + tainted_call_info *info = new tainted_call_info (field, fndecl, loc); + eg->add_edge (eg->get_origin (), enode, NULL, info); +} + +/* Callback for walk_tree for finding callbacks within initializers; + ensure that any callback initializer where the corresponding field is + marked with '__attribute__((tainted))' is treated as an entrypoint to the + analysis, special-casing that the inputs to the callback are + untrustworthy. */ + +static tree +add_any_callbacks (tree *tp, int *, void *data) +{ + exploded_graph *eg = (exploded_graph *)data; + if (TREE_CODE (*tp) == CONSTRUCTOR) + { + /* Find fields with the "tainted" attribute. + walk_tree only walks the values, not the index values; + look at the index values. */ + unsigned HOST_WIDE_INT idx; + constructor_elt *ce; + + for (idx = 0; vec_safe_iterate (CONSTRUCTOR_ELTS (*tp), idx, &ce); + idx++) + if (ce->index && TREE_CODE (ce->index) == FIELD_DECL) + if (lookup_attribute ("tainted", DECL_ATTRIBUTES (ce->index))) + { + tree value = ce->value; + if (TREE_CODE (value) == ADDR_EXPR + && TREE_CODE (TREE_OPERAND (value, 0)) == FUNCTION_DECL) + add_tainted_callback (eg, ce->index, TREE_OPERAND (value, 0), + EXPR_LOCATION (value)); + } + } + + return NULL_TREE; +} + /* Add initial nodes to EG, with entrypoints for externally-callable functions. */ @@ -2648,6 +2950,17 @@ exploded_graph::build_initial_worklist () logger->log ("did not create enode for %qE entrypoint", fun->decl); } } + + /* Find callbacks that are reachable from global initializers. */ + varpool_node *vpnode; + FOR_EACH_VARIABLE (vpnode) + { + tree decl = vpnode->decl; + tree init = DECL_INITIAL (decl); + if (!init) + continue; + walk_tree (&init, add_any_callbacks, this, NULL); + } } /* The main loop of the analysis. diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c index 9e03156de5e..835ba6e0e8c 100644 --- a/gcc/c-family/c-attribs.c +++ b/gcc/c-family/c-attribs.c @@ -117,6 +117,7 @@ static tree handle_no_profile_instrument_function_attribute (tree *, tree, tree, int, bool *); static tree handle_malloc_attribute (tree *, tree, tree, int, bool *); static tree handle_dealloc_attribute (tree *, tree, tree, int, bool *); +static tree handle_tainted_attribute (tree *, tree, tree, int, bool *); static tree handle_returns_twice_attribute (tree *, tree, tree, int, bool *); static tree handle_no_limit_stack_attribute (tree *, tree, tree, int, bool *); @@ -569,6 +570,8 @@ const struct attribute_spec c_common_attribute_table[] = handle_objc_nullability_attribute, NULL }, { "*dealloc", 1, 2, true, false, false, false, handle_dealloc_attribute, NULL }, + { "tainted", 0, 0, true, false, false, false, + handle_tainted_attribute, NULL }, { NULL, 0, 0, false, false, false, false, NULL, NULL } }; @@ -5857,6 +5860,39 @@ handle_objc_nullability_attribute (tree *node, tree name, tree args, return NULL_TREE; } +/* Handle a "tainted" attribute; arguments as in + struct attribute_spec.handler. */ + +static tree +handle_tainted_attribute (tree *node, tree name, tree, int, + bool *no_add_attrs) +{ + if (TREE_CODE (*node) != FUNCTION_DECL + && TREE_CODE (*node) != FIELD_DECL) + { + warning (OPT_Wattributes, "%qE attribute ignored; valid only " + "for functions and function pointer fields", + name); + *no_add_attrs = true; + return NULL_TREE; + } + + if (TREE_CODE (*node) == FIELD_DECL + && !(TREE_CODE (TREE_TYPE (*node)) == POINTER_TYPE + && TREE_CODE (TREE_TYPE (TREE_TYPE (*node))) == FUNCTION_TYPE)) + { + warning (OPT_Wattributes, "%qE attribute ignored;" + " field must be a function pointer", + name); + *no_add_attrs = true; + return NULL_TREE; + } + + *no_add_attrs = false; /* OK */ + + return NULL_TREE; +} + /* Attempt to partially validate a single attribute ATTR as if it were to be applied to an entity OPER. */ diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index 5a6ef464779..826bbd48e7e 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -2465,7 +2465,8 @@ variable declarations (@pxref{Variable Attributes}), labels (@pxref{Label Attributes}), enumerators (@pxref{Enumerator Attributes}), statements (@pxref{Statement Attributes}), -and types (@pxref{Type Attributes}). +types (@pxref{Type Attributes}), +and on field declarations (for @code{tainted}). There is some overlap between the purposes of attributes and pragmas (@pxref{Pragmas,,Pragmas Accepted by GCC}). It has been @@ -3977,6 +3978,25 @@ addition to creating a symbol version (as if @code{"@var{name2}@@@var{nodename}"} was used) the version will be also used to resolve @var{name2} by the linker. +@item tainted +@cindex @code{tainted} function attribute +The @code{tainted} attribute is used to specify that a function is called +in a way that requires sanitization of its arguments, such as a system +call in an operating system kernel. Such a function can be considered part +of the ``attack surface'' of the program. The attribute can be used both +on function declarations, and on field declarations containing function +pointers. In the latter case, any function used as an initializer of +such a callback field will be treated as tainted. + +The analyzer will pay particular attention to such functions when both +@option{-fanalyzer} and @option{-fanalyzer-checker=taint} are supplied, +potentially issuing warnings guarded by +@option{-Wanalyzer-exposure-through-uninit-copy}, +@option{-Wanalyzer-tainted-allocation-size}, +@option{-Wanalyzer-tainted-array-index}, +@option{Wanalyzer-tainted-offset}, +and @option{Wanalyzer-tainted-size}. + @item target_clones (@var{options}) @cindex @code{target_clones} function attribute The @code{target_clones} attribute is used to specify that a function diff --git a/gcc/testsuite/gcc.dg/analyzer/attr-tainted-1.c b/gcc/testsuite/gcc.dg/analyzer/attr-tainted-1.c new file mode 100644 index 00000000000..cc4d5900372 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/attr-tainted-1.c @@ -0,0 +1,88 @@ +// TODO: remove need for this option +/* { dg-additional-options "-fanalyzer-checker=taint" } */ + +#include "analyzer-decls.h" + +struct arg_buf +{ + int i; + int j; +}; + +/* Example of marking a function as tainted. */ + +void __attribute__((tainted)) +test_1 (int i, void *p, char *q) +{ + /* There should be a single enode, + for the "tainted" entry to the function. */ + __analyzer_dump_exploded_nodes (0); /* { dg-warning "1 processed enode" } */ + + __analyzer_dump_state ("taint", i); /* { dg-warning "state: 'tainted'" } */ + __analyzer_dump_state ("taint", p); /* { dg-warning "state: 'tainted'" } */ + __analyzer_dump_state ("taint", q); /* { dg-warning "state: 'tainted'" } */ + __analyzer_dump_state ("taint", *q); /* { dg-warning "state: 'tainted'" } */ + + struct arg_buf *args = p; + __analyzer_dump_state ("taint", args->i); /* { dg-warning "state: 'tainted'" } */ + __analyzer_dump_state ("taint", args->j); /* { dg-warning "state: 'tainted'" } */ +} + +/* Example of marking a callback field as tainted. */ + +struct s2 +{ + void (*cb) (int, void *, char *) + __attribute__((tainted)); +}; + +/* Function not marked as tainted. */ + +void +test_2a (int i, void *p, char *q) +{ + /* There should be a single enode, + for the normal entry to the function. */ + __analyzer_dump_exploded_nodes (0); /* { dg-warning "1 processed enode" } */ + + __analyzer_dump_state ("taint", i); /* { dg-warning "state: 'start'" } */ + __analyzer_dump_state ("taint", p); /* { dg-warning "state: 'start'" } */ + __analyzer_dump_state ("taint", q); /* { dg-warning "state: 'start'" } */ + + struct arg_buf *args = p; + __analyzer_dump_state ("taint", args->i); /* { dg-warning "state: 'start'" } */ + __analyzer_dump_state ("taint", args->j); /* { dg-warning "state: 'start'" } */ +} + +/* Function referenced via t2b.cb, marked as "tainted". */ + +void +test_2b (int i, void *p, char *q) +{ + /* There should be two enodes + for the direct call, and the "tainted" entry to the function. */ + __analyzer_dump_exploded_nodes (0); /* { dg-warning "2 processed enodes" } */ +} + +/* Callback used via t2c.cb, marked as "tainted". */ +void +__analyzer_test_2c (int i, void *p, char *q) +{ + /* There should be a single enode, + for the "tainted" entry to the function. */ + __analyzer_dump_exploded_nodes (0); /* { dg-warning "1 processed enode" } */ + + __analyzer_dump_state ("taint", i); /* { dg-warning "state: 'tainted'" } */ + __analyzer_dump_state ("taint", p); /* { dg-warning "state: 'tainted'" } */ + __analyzer_dump_state ("taint", q); /* { dg-warning "state: 'tainted'" } */ +} + +struct s2 t2b = +{ + .cb = test_2b +}; + +struct s2 t2c = +{ + .cb = __analyzer_test_2c +}; diff --git a/gcc/testsuite/gcc.dg/analyzer/attr-tainted-misuses.c b/gcc/testsuite/gcc.dg/analyzer/attr-tainted-misuses.c new file mode 100644 index 00000000000..6f4cbc82efb --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/attr-tainted-misuses.c @@ -0,0 +1,6 @@ +int not_a_fn __attribute__ ((tainted)); /* { dg-warning "'tainted' attribute ignored; valid only for functions and function pointer fields" } */ + +struct s +{ + int f __attribute__ ((tainted)); /* { dg-warning "'tainted' attribute ignored; field must be a function pointer" } */ +}; diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-2210-1.c b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-2210-1.c new file mode 100644 index 00000000000..fe6c7ebbb1f --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2011-2210-1.c @@ -0,0 +1,93 @@ +/* "The osf_getsysinfo function in arch/alpha/kernel/osf_sys.c in the + Linux kernel before 2.6.39.4 on the Alpha platform does not properly + restrict the data size for GSI_GET_HWRPB operations, which allows + local users to obtain sensitive information from kernel memory via + a crafted call." + + Fixed in 3d0475119d8722798db5e88f26493f6547a4bb5b on linux-2.6.39.y + in linux-stable. */ + +// TODO: remove need for this option: +/* { dg-additional-options "-fanalyzer-checker=taint" } */ + +#include "analyzer-decls.h" +#include "test-uaccess.h" + +/* Adapted from include/linux/linkage.h. */ + +#define asmlinkage + +/* Adapted from include/linux/syscalls.h. */ + +#define __SC_DECL1(t1, a1) t1 a1 +#define __SC_DECL2(t2, a2, ...) t2 a2, __SC_DECL1(__VA_ARGS__) +#define __SC_DECL3(t3, a3, ...) t3 a3, __SC_DECL2(__VA_ARGS__) +#define __SC_DECL4(t4, a4, ...) t4 a4, __SC_DECL3(__VA_ARGS__) +#define __SC_DECL5(t5, a5, ...) t5 a5, __SC_DECL4(__VA_ARGS__) +#define __SC_DECL6(t6, a6, ...) t6 a6, __SC_DECL5(__VA_ARGS__) + +#define SYSCALL_DEFINEx(x, sname, ...) \ + __SYSCALL_DEFINEx(x, sname, __VA_ARGS__) + +#define SYSCALL_DEFINE(name) asmlinkage long sys_##name +#define __SYSCALL_DEFINEx(x, name, ...) \ + asmlinkage __attribute__((tainted)) \ + long sys##name(__SC_DECL##x(__VA_ARGS__)) + +#define SYSCALL_DEFINE5(name, ...) SYSCALL_DEFINEx(5, _##name, __VA_ARGS__) + +/* Adapted from arch/alpha/include/asm/hwrpb.h. */ + +struct hwrpb_struct { + unsigned long phys_addr; /* check: physical address of the hwrpb */ + unsigned long id; /* check: "HWRPB\0\0\0" */ + unsigned long revision; + unsigned long size; /* size of hwrpb */ + /* [...snip...] */ +}; + +extern struct hwrpb_struct *hwrpb; + +/* Adapted from arch/alpha/kernel/osf_sys.c. */ + +SYSCALL_DEFINE5(osf_getsysinfo, unsigned long, op, void __user *, buffer, + unsigned long, nbytes, int __user *, start, void __user *, arg) +{ + /* [...snip...] */ + + __analyzer_dump_state ("taint", nbytes); /* { dg-warning "tainted" } */ + + /* TODO: should have an event explaining why "nbytes" is treated as + attacker-controlled. */ + + /* case GSI_GET_HWRPB: */ + if (nbytes < sizeof(*hwrpb)) + return -1; + + __analyzer_dump_state ("taint", nbytes); /* { dg-warning "has_lb" } */ + + if (copy_to_user(buffer, hwrpb, nbytes) != 0) /* { dg-warning "use of attacker-controlled value 'nbytes' as size without upper-bounds checking" } */ + return -2; + + return 1; + + /* [...snip...] */ +} + +/* With the fix for the sense of the size comparison. */ + +SYSCALL_DEFINE5(osf_getsysinfo_fixed, unsigned long, op, void __user *, buffer, + unsigned long, nbytes, int __user *, start, void __user *, arg) +{ + /* [...snip...] */ + + /* case GSI_GET_HWRPB: */ + if (nbytes > sizeof(*hwrpb)) + return -1; + if (copy_to_user(buffer, hwrpb, nbytes) != 0) /* { dg-bogus "attacker-controlled" } */ + return -2; + + return 1; + + /* [...snip...] */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2020-13143-1.c b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2020-13143-1.c new file mode 100644 index 00000000000..0b9a94a8d6c --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2020-13143-1.c @@ -0,0 +1,38 @@ +/* See notes in this header. */ +#include "taint-CVE-2020-13143.h" + +// TODO: remove need for this option +/* { dg-additional-options "-fanalyzer-checker=taint" } */ + +struct configfs_attribute { + /* [...snip...] */ + ssize_t (*store)(struct config_item *, const char *, size_t) /* { dg-message "\\(1\\) field 'store' of 'struct configfs_attribute' is marked with '__attribute__\\(\\(tainted\\)\\)'" } */ + __attribute__((tainted)); /* (this is added). */ +}; +static inline struct gadget_info *to_gadget_info(struct config_item *item) +{ + return container_of(to_config_group(item), struct gadget_info, group); +} + +static ssize_t gadget_dev_desc_UDC_store(struct config_item *item, + const char *page, size_t len) +{ + struct gadget_info *gi = to_gadget_info(item); + char *name; + int ret; + +#if 0 + /* FIXME: this is the fix. */ + if (strlen(page) < len) + return -EOVERFLOW; +#endif + + name = kstrdup(page, GFP_KERNEL); + if (!name) + return -ENOMEM; + if (name[len - 1] == '\n') /* { dg-warning "use of attacker-controlled value 'len \[^\n\r\]+' as offset without upper-bounds checking" } */ + name[len - 1] = '\0'; /* { dg-warning "use of attacker-controlled value 'len \[^\n\r\]+' as offset without upper-bounds checking" } */ + /* [...snip...] */ \ +} + +CONFIGFS_ATTR(gadget_dev_desc_, UDC); /* { dg-message "\\(2\\) function 'gadget_dev_desc_UDC_store' used as initializer for field 'store' marked with '__attribute__\\(\\(tainted\\)\\)'" } */ diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2020-13143-2.c b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2020-13143-2.c new file mode 100644 index 00000000000..e05da9276c1 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2020-13143-2.c @@ -0,0 +1,32 @@ +/* See notes in this header. */ +#include "taint-CVE-2020-13143.h" + +// TODO: remove need for this option +/* { dg-additional-options "-fanalyzer-checker=taint" } */ + +struct configfs_attribute { + /* [...snip...] */ + ssize_t (*store)(struct config_item *, const char *, size_t) /* { dg-message "\\(1\\) field 'store' of 'struct configfs_attribute' is marked with '__attribute__\\(\\(tainted\\)\\)'" } */ + __attribute__((tainted)); /* (this is added). */ +}; + +/* Highly simplified version. */ + +static ssize_t gadget_dev_desc_UDC_store(struct config_item *item, + const char *page, size_t len) +{ + /* TODO: ought to have state_change_event talking about where the tainted value comes from. */ + + char *name; + /* [...snip...] */ + + name = kstrdup(page, GFP_KERNEL); + if (!name) + return -ENOMEM; + if (name[len - 1] == '\n') /* { dg-warning "use of attacker-controlled value 'len \[^\n\r\]+' as offset without upper-bounds checking" } */ + name[len - 1] = '\0'; /* { dg-warning "use of attacker-controlled value 'len \[^\n\r\]+' as offset without upper-bounds checking" } */ + /* [...snip...] */ + return 0; +} + +CONFIGFS_ATTR(gadget_dev_desc_, UDC); /* { dg-message "\\(2\\) function 'gadget_dev_desc_UDC_store' used as initializer for field 'store' marked with '__attribute__\\(\\(tainted\\)\\)'" } */ diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2020-13143.h b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2020-13143.h new file mode 100644 index 00000000000..0ba023539af --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/taint-CVE-2020-13143.h @@ -0,0 +1,91 @@ +/* Shared header for the various taint-CVE-2020-13143.h tests. + + "gadget_dev_desc_UDC_store in drivers/usb/gadget/configfs.c in the + Linux kernel 3.16 through 5.6.13 relies on kstrdup without considering + the possibility of an internal '\0' value, which allows attackers to + trigger an out-of-bounds read, aka CID-15753588bcd4." + + Fixed by 15753588bcd4bbffae1cca33c8ced5722477fe1f on linux-5.7.y + in linux-stable. */ + +// TODO: remove need for this option +/* { dg-additional-options "-fanalyzer-checker=taint" } */ + +#include + +/* Adapted from include/uapi/asm-generic/posix_types.h */ + +typedef unsigned int __kernel_size_t; +typedef int __kernel_ssize_t; + +/* Adapted from include/linux/types.h */ + +//typedef __kernel_size_t size_t; +typedef __kernel_ssize_t ssize_t; + +/* Adapted from include/linux/kernel.h */ + +#define container_of(ptr, type, member) ({ \ + void *__mptr = (void *)(ptr); \ + /* [...snip...] */ \ + ((type *)(__mptr - offsetof(type, member))); }) + +/* Adapted from include/linux/configfs.h */ + +struct config_item { + /* [...snip...] */ +}; + +struct config_group { + struct config_item cg_item; + /* [...snip...] */ +}; + +static inline struct config_group *to_config_group(struct config_item *item) +{ + return item ? container_of(item,struct config_group,cg_item) : NULL; +} + +#define CONFIGFS_ATTR(_pfx, _name) \ +static struct configfs_attribute _pfx##attr_##_name = { \ + /* [...snip...] */ \ + .store = _pfx##_name##_store, \ +} + +/* Adapted from include/linux/compiler.h */ + +#define __force + +/* Adapted from include/asm-generic/errno-base.h */ + +#define ENOMEM 12 /* Out of memory */ + +/* Adapted from include/linux/types.h */ + +#define __bitwise__ +typedef unsigned __bitwise__ gfp_t; + +/* Adapted from include/linux/gfp.h */ + +#define ___GFP_WAIT 0x10u +#define ___GFP_IO 0x40u +#define ___GFP_FS 0x80u +#define __GFP_WAIT ((__force gfp_t)___GFP_WAIT) +#define __GFP_IO ((__force gfp_t)___GFP_IO) +#define __GFP_FS ((__force gfp_t)___GFP_FS) +#define GFP_KERNEL (__GFP_WAIT | __GFP_IO | __GFP_FS) + +/* Adapted from include/linux/compiler_attributes.h */ + +#define __malloc __attribute__((__malloc__)) + +/* Adapted from include/linux/string.h */ + +extern char *kstrdup(const char *s, gfp_t gfp) __malloc; + +/* Adapted from drivers/usb/gadget/configfs.c */ + +struct gadget_info { + struct config_group group; + /* [...snip...] */ \ +}; diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-alloc-3.c b/gcc/testsuite/gcc.dg/analyzer/taint-alloc-3.c new file mode 100644 index 00000000000..4c567b2ffdf --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/taint-alloc-3.c @@ -0,0 +1,21 @@ +// TODO: remove need for this option: +/* { dg-additional-options "-fanalyzer-checker=taint" } */ + +#include "analyzer-decls.h" +#include +#include +#include + +/* malloc with tainted size from a syscall. */ + +void *p; + +void __attribute__((tainted)) +test_1 (size_t sz) /* { dg-message "\\(1\\) function 'test_1' marked with '__attribute__\\(\\(tainted\\)\\)'" } */ +{ + /* TODO: should have a message saying why "sz" is tainted, e.g. + "treating 'sz' as attacker-controlled because 'test_1' is marked with '__attribute__((tainted))'" */ + + p = malloc (sz); /* { dg-warning "use of attacker-controlled value 'sz' as allocation size without upper-bounds checking" "warning" } */ + /* { dg-message "\\(\[0-9\]+\\) use of attacker-controlled value 'sz' as allocation size without upper-bounds checking" "final event" { target *-*-* } .-1 } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-alloc-4.c b/gcc/testsuite/gcc.dg/analyzer/taint-alloc-4.c new file mode 100644 index 00000000000..f52cafcd71d --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/taint-alloc-4.c @@ -0,0 +1,31 @@ +// TODO: remove need for this option: +/* { dg-additional-options "-fanalyzer-checker=taint" } */ + +#include "analyzer-decls.h" +#include +#include +#include + +/* malloc with tainted size from a syscall. */ + +struct arg_buf +{ + size_t sz; +}; + +void *p; + +void __attribute__((tainted)) +test_1 (void *data) /* { dg-message "\\(1\\) function 'test_1' marked with '__attribute__\\(\\(tainted\\)\\)'" } */ +{ + /* we should treat pointed-to-structs as tainted. */ + __analyzer_dump_state ("taint", data); /* { dg-warning "state: 'tainted'" } */ + + struct arg_buf *args = data; + + __analyzer_dump_state ("taint", args); /* { dg-warning "state: 'tainted'" } */ + __analyzer_dump_state ("taint", args->sz); /* { dg-warning "state: 'tainted'" } */ + + p = malloc (args->sz); /* { dg-warning "use of attacker-controlled value '\\*args.sz' as allocation size without upper-bounds checking" "warning" } */ + /* { dg-message "\\(\[0-9\]+\\) use of attacker-controlled value '\\*args.sz' as allocation size without upper-bounds checking" "final event" { target *-*-* } .-1 } */ +}