[Bug analyzer/106235] New: RFE: -fanalyzer could complain about tainted data triggering assertion failure

[Bug analyzer/106235] New: RFE: -fanalyzer could complain about tainted data triggering assertion failure

[dmalcolm at gcc dot gnu.org]

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

            Bug ID: 106235
           Summary: RFE: -fanalyzer could complain about tainted data
                    triggering assertion failure
           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
  Target Milestone: ---

CWE-617: Reachable Assertion:
  https://cwe.mitre.org/data/definitions/617.html
"The product contains an assert() or similar statement that can be triggered by
an attacker, which leads to an application exit or other behavior that is more
severe than necessary."

(e.g. remote triggering of denial-of-service)


Perhaps -fanalyzer could identify assertion failure routines, and see if
tainted data is used in an assertion.  Presumably we'd want to see if a
conditional guarding an assertion handler involves tainted data.

Not sure if this is fully implementable; e.g. what to do about non-trivial
conditionals?  (and how much can we reconstruct about "is this an assertion" vs
"is this a regular conditional" given how late we run)

[Bug analyzer/106235] RFE: -fanalyzer could complain about tainted data triggering assertion failure

[dmalcolm at gcc dot gnu.org]

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

--- Comment #1 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Juliet 1.3 has various testcases for this in
  C/testcases/CWE617_Reachable_Assertion/

[Bug analyzer/106235] RFE: -fanalyzer could complain about tainted data triggering assertion failure

[dmalcolm at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2022-11-13
     Ever confirmed|0                           |1

--- Comment #2 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Testing a patch for this...

[Bug analyzer/106235] RFE: -fanalyzer could complain about tainted data triggering assertion failure

[cvs-commit at gcc dot gnu.org]

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

--- 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:d777b38cde91a87f2345dcd13901862a9513562a

commit r13-3947-gd777b38cde91a87f2345dcd13901862a9513562a
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Sun Nov 13 17:53:23 2022 -0500

    analyzer: new warning: -Wanalyzer-tainted-assertion [PR106235]

    This patch adds a new -Wanalyzer-tainted-assertion warning to
    -fanalyzer's "taint" mode (which also requires -fanalyzer-checker=taint).

    It complains about attacker-controlled values being used in assertions,
    or in any expression affecting control flow that guards a "noreturn"
    function.  As noted in the docs part of the patch, in such cases:

      - when assertion-checking is enabled: an attacker could trigger
        a denial of service by injecting an assertion failure

      - when assertion-checking is disabled, such as by defining NDEBUG,
        an attacker could inject data that subverts the process, since it
        presumably violates a precondition that is being assumed by the code.

    For example, given:

    #include <assert.h>

    int __attribute__((tainted_args))
    test_tainted_assert (int n)
    {
      assert (n > 0);
      return n * n;
    }

    compiling with
      -fanalyzer -fanalyzer-checker=taint
    gives:

    t.c: In function 'test_tainted_assert':
    t.c:6:3: warning: use of attacked-controlled value in condition for
assertion [CWE-617] [-Wanalyzer-tainted-assertion]
        6 |   assert (n > 0);
          |   ^~~~~~
      'test_tainted_assert': event 1
        |
        |    4 | test_tainted_assert (int n)
        |      | ^~~~~~~~~~~~~~~~~~~
        |      | |
        |      | (1) function 'test_tainted_assert' marked with
'__attribute__((tainted_args))'
        |
        +--> 'test_tainted_assert': event 2
               |
               |    4 | test_tainted_assert (int n)
               |      | ^~~~~~~~~~~~~~~~~~~
               |      | |
               |      | (2) entry to 'test_tainted_assert'
               |
             'test_tainted_assert': events 3-6
               |
               |/usr/include/assert.h:106:10:
               |  106 |       if (expr)                                        
                \
               |      |          ^
               |      |          |
               |      |          (3) use of attacker-controlled value for
control flow
               |      |          (4) following 'false' branch (when 'n <=
0')...
               |......
               |  109 |         __assert_fail (#expr, __FILE__, __LINE__,
__ASSERT_FUNCTION);   \
               |      |         ~~~~~~~~~~~~~
               |      |         |
               |      |         (5) ...to here
               |      |         (6) treating '__assert_fail' as an assertion
failure handler due to '__attribute__((__noreturn__))'
               |

    The testcases have various examples for BUG and BUG_ON from the
    Linux kernel; there, the diagnostic treats "panic" as an assertion
    failure handler, due to '__attribute__((__noreturn__))'.

    gcc/analyzer/ChangeLog:
            PR analyzer/106235
            * analyzer.opt (Wanalyzer-tainted-assertion): New.
            * checker-path.cc (checker_path::fixup_locations): Pass false to
            pending_diagnostic::fixup_location.
            * diagnostic-manager.cc (get_emission_location): Pass true to
            pending_diagnostic::fixup_location.
            * pending-diagnostic.cc (pending_diagnostic::fixup_location): Add
            bool param.
            * pending-diagnostic.h (pending_diagnostic::fixup_location): Add
            bool param to decl.
            * sm-taint.cc (taint_state_machine::m_tainted_control_flow): New.
            (taint_diagnostic::describe_state_change): Drop "final".
            (class tainted_assertion): New.
            (taint_state_machine::taint_state_machine): Initialize
            m_tainted_control_flow.
            (taint_state_machine::alt_get_inherited_state): Support
            comparisons being tainted, based on their arguments.
            (is_assertion_failure_handler_p): New.
            (taint_state_machine::on_stmt): Complain about calls to assertion
            failure handlers guarded by an attacker-controller conditional.
            Detect attacker-controlled gcond conditionals and gswitch index
            values.
            (taint_state_machine::check_control_flow_arg_for_taint): New.

    gcc/ChangeLog:
            PR analyzer/106235
            * doc/gcc/gcc-command-options/option-summary.rst: Add
            -Wno-analyzer-tainted-assertion.
            *
doc/gcc/gcc-command-options/options-that-control-static-analysis.rst:
            Add -Wno-analyzer-tainted-assertion.

    gcc/testsuite/ChangeLog:
            PR analyzer/106235
            * gcc.dg/analyzer/taint-assert-BUG_ON.c: New test.
            * gcc.dg/analyzer/taint-assert-macro-expansion.c: New test.
            * gcc.dg/analyzer/taint-assert.c: New test.
            * gcc.dg/analyzer/taint-assert-system-header.c: New test.
            * gcc.dg/analyzer/test-assert.h: New header.
            * gcc.dg/plugin/analyzer_gil_plugin.c
            (gil_diagnostic::fixup_location): Add bool param.

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

[Bug analyzer/106235] RFE: -fanalyzer could complain about tainted data triggering assertion failure

[dmalcolm at gcc dot gnu.org]

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

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

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

--- Comment #4 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Implemented for GCC 13 by the above patch.

[Bug analyzer/106235] RFE: -fanalyzer could complain about tainted data triggering assertion failure

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |13.0