[Bug analyzer/103940] New: RFE: check -Wanalyzer-tainted-size on external fns with attribute ((access)) with a size-index

[Bug analyzer/103940] New: RFE: check -Wanalyzer-tainted-size on external fns with attribute ((access)) with a size-index

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103940

            Bug ID: 103940
           Summary: RFE: check -Wanalyzer-tainted-size on external fns
                    with attribute ((access)) with a size-index
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: analyzer
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: dmalcolm at gcc dot gnu.org
                CC: msebor at gcc dot gnu.org
  Target Milestone: ---

GCC 10 gained the "access" function and type attribute, which optionally can
take a size-index param:
  https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html

-fanalyzer in trunk (for GCC 12) has gained a -Wanalyzer-tainted-size to
complain about attacker-controlled size values, but it's currently only used
deep inside the region-model code when handling the hardcoded known behavior of
certain functions (memset, IIRC).

Filing this as a reminder that we could probably also issue
-Wanalyzer-tainted-size if an attacker-controlled value is passed without
sanitization as a size to an access-annotated external function.

Martin: does this sound like a good idea?

[Bug analyzer/103940] RFE: check -Wanalyzer-tainted-size on external fns with attribute ((access)) with a size-index

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103940

--- Comment #1 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Note that -fanalyzer-checker=taint is currently required in addition to
-fanalyzer to use this warning, due to scaling issues (see bug 103533).

[Bug analyzer/103940] RFE: check -Wanalyzer-tainted-size on external fns with attribute ((access)) with a size-index

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103940

--- Comment #2 from Martin Sebor <msebor at gcc dot gnu.org> ---
It makes sense to me, David.

[Bug analyzer/103940] RFE: check -Wanalyzer-tainted-size on external fns with attribute ((access)) with a size-index

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103940

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>:

https://gcc.gnu.org/g:2c16dfe6268eeeb4b7924ff423e274fa00894a4d

commit r12-6526-g2c16dfe6268eeeb4b7924ff423e274fa00894a4d
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Tue Jan 11 15:57:39 2022 -0500

    analyzer: complain about tainted sizes with "access" attribute [PR103940]

    GCC 10 gained the "access" function and type attribute, which
    optionally can take a size-index param:
      https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html

    -fanalyzer in trunk (for GCC 12) has gained a -Wanalyzer-tainted-size to
    complain about attacker-controlled size values, but this was only being
    used deep inside the region-model code when handling the hardcoded known
    behavior of certain functions (memset, IIRC).

    This patch extends -Wanalyzer-tainted-size to also complain about
    unsanitized attacker-controlled values being passed to function
    parameters marked as a size via the "access" attribute.

    Note that -fanalyzer-checker=taint is currently required in
    addition to -fanalyzer to use this warning, due to scaling issues
    (see bug 103533).

    gcc/analyzer/ChangeLog:
            PR analyzer/103940
            * engine.cc (impl_sm_context::impl_sm_context): Add
            "unknown_side_effects" param and use it to initialize
            new m_unknown_side_effects field.
            (impl_sm_context::unknown_side_effects_p): New.
            (impl_sm_context::m_unknown_side_effects): New.
            (exploded_node::on_stmt): Pass unknown_side_effects to sm_ctxt
            ctor.
            * sm-taint.cc: Include "stringpool.h" and "attribs.h".
            (tainted_size::tainted_size): Drop "dir" param.
            (tainted_size::get_kind): Drop "FINAL".
            (tainted_size::emit): Likewise.
            (tainted_size::m_dir): Drop unused field.
            (class tainted_access_attrib_size): New subclass.
            (taint_state_machine::on_stmt): Call check_for_tainted_size_arg on
            external functions with unknown side effects.
            (taint_state_machine::check_for_tainted_size_arg): New.
            (region_model::check_region_for_taint): Drop "dir" param from
            tainted_size ctor.
            * sm.h (sm_context::unknown_side_effects_p): New.

    gcc/testsuite/ChangeLog:
            PR analyzer/103940
            * gcc.dg/analyzer/taint-size-access-attr-1.c: New test.

    Signed-off-by: David Malcolm <dmalcolm@redhat.com>

[Bug analyzer/103940] RFE: check -Wanalyzer-tainted-size on external fns with attribute ((access)) with a size-index

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103940

David Malcolm <dmalcolm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #4 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Should be fixed by the above commit.