Bug 106140 - RFE: analyzer could complain about misuses of socket APIs
Summary: RFE: analyzer could complain about misuses of socket APIs
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: analyzer (show other bugs)
Version: 12.0
: P3 normal
Target Milestone: 13.0
Assignee: David Malcolm
URL:
Keywords:
Depends on: 106302
Blocks: analyzer-fd
  Show dependency treegraph
 
Reported: 2022-06-29 22:14 UTC by David Malcolm
Modified: 2022-11-28 22:20 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2022-10-31 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Malcolm 2022-06-29 22:14:37 UTC
POSIX has numerous API entrypoints for working with sockets, and, presumably, numerous ways of misusing them.  They're expressed in terms of file-descriptors, and so checking them would interact with PR 106003.

One specific example: the Juliet 1.3 testsuite 
  https://samate.nist.gov/SARD/test-suites
has a subdirectory:

  C/testcases/CWE666_Operation_on_Resource_in_Wrong_Phase_of_Lifetime/

which has e.g. tests that sockets have calls to:
  bind then listen then accept  (good)
rather than, say:
  accept then bind then listen  (bad)

This could be modeled as part of a state machine, possibly as part of the FD state machine.
Comment 1 Eric Gallager 2022-07-23 01:13:08 UTC
(In reply to David Malcolm from comment #0)
> POSIX has numerous API entrypoints for working with sockets, and,
> presumably, numerous ways of misusing them.  They're expressed in terms of
> file-descriptors, and so checking them would interact with PR 106003.
> 

Oops wait that's just the analyzer-fd meta-bug; guess I don't need to put it under "See Also" after all...
Comment 2 David Malcolm 2022-10-31 14:15:06 UTC
I have an implementation of this, but it needs to be able to grok SOCK_DGRAM vs SOCK_STREAM, which is another instance of PR 106302.
Comment 3 David Malcolm 2022-11-08 03:03:59 UTC
Candidate patch posted here:
  [PATCH] analyzer: add warnings relating to sockets [PR106140]
     https://gcc.gnu.org/pipermail/gcc-patches/2022-November/605281.html
Comment 4 GCC Commits 2022-11-15 18:59:21 UTC
The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>:

https://gcc.gnu.org/g:86a90006864840c2e222d46ead551850caba184b

commit r13-4074-g86a90006864840c2e222d46ead551850caba184b
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Tue Nov 15 13:53:42 2022 -0500

    analyzer: add warnings relating to sockets [PR106140]
    
    This patch generalizes the analyzer's file descriptor state machine
    so that it tracks the states of sockets.
    
    It adds two new warnings relating to misuses of socket APIs:
    * -Wanalyzer-fd-phase-mismatch (e.g. calling 'accept' on a socket
    before calling 'listen' on it)
    * -Wanalyzer-fd-type-mismatch (e.g. using a stream socket operation
    on a datagram socket)
    
    gcc/analyzer/ChangeLog:
            PR analyzer/106140
            * analyzer-language.cc (on_finish_translation_unit): Stash named
            constants "SOCK_STREAM" and "SOCK_DGRAM".
            * analyzer.opt (Wanalyzer-fd-phase-mismatch): New.
            (Wanalyzer-fd-type-mismatch): New.
            * engine.cc (impl_region_model_context::get_state_map_by_name):
            Add "out_sm_context" param.  Allow out_sm_idx to be NULL.
            * exploded-graph.h
            (impl_region_model_context::get_state_map_by_name):
            Add "out_sm_context" param.
            * region-model-impl-calls.cc (region_model::impl_call_accept): New.
            (region_model::impl_call_bind): New.
            (region_model::impl_call_connect): New.
            (region_model::impl_call_listen): New.
            (region_model::impl_call_socket): New.
            * region-model.cc (region_model::on_call_pre): Special-case
            "bind".
            (region_model::on_call_post): Special-case "accept", "bind",
            "connect", "listen", and "socket".
            * region-model.h (region_model::impl_call_accept): New decl.
            (region_model::impl_call_bind): New decl.
            (region_model::impl_call_connect): New decl.
            (region_model::impl_call_listen): New decl.
            (region_model::impl_call_socket): New decl.
            (region_model::on_socket): New decl.
            (region_model::on_bind): New decl.
            (region_model::on_listen): New decl.
            (region_model::on_accept): New decl.
            (region_model::on_connect): New decl.
            (region_model::add_constraint): Make public.
            (region_model::check_for_poison): Make public.
            (region_model_context::get_state_map_by_name): Add out_sm_context param.
            (region_model_context::get_fd_map): Likewise.
            (region_model_context::get_malloc_map): Likewise.
            (region_model_context::get_taint_map): Likewise.
            (noop_region_model_context::get_state_map_by_name): Likewise.
            (region_model_context_decorator::get_state_map_by_name): Likewise.
            * sm-fd.cc: Include "analyzer/supergraph.h" and
            "analyzer/analyzer-language.h".
            (enum expected_phase): New enum.
            (fd_state_machine::m_new_datagram_socket): New.
            (fd_state_machine::m_new_stream_socket): New.
            (fd_state_machine::m_new_unknown_socket): New.
            (fd_state_machine::m_bound_datagram_socket): New.
            (fd_state_machine::m_bound_stream_socket): New.
            (fd_state_machine::m_bound_unknown_socket): New.
            (fd_state_machine::m_listening_stream_socket): New.
            (fd_state_machine::m_m_connected_stream_socket): New.
            (fd_state_machine::m_SOCK_STREAM): New.
            (fd_state_machine::m_SOCK_DGRAM): New.
            (fd_diagnostic::describe_state_change): Handle socket states.
            (fd_diagnostic::get_meaning_for_state_change): Likewise.
            (class fd_phase_mismatch): New.
            (enum expected_type): New enum.
            (class fd_type_mismatch): New.
            (fd_state_machine::fd_state_machine): Initialize new states and
            stashed named constants.
            (fd_state_machine::is_socket_fd_p): New.
            (fd_state_machine::is_datagram_socket_fd_p): New.
            (fd_state_machine::is_stream_socket_fd_p): New.
            (fd_state_machine::on_close): Handle the socket states.
            (fd_state_machine::check_for_open_fd): Complain about fncalls on
            sockets in the wrong phase.  Support socket FDs.
            (add_constraint_ge_zero): New.
            (fd_state_machine::get_state_for_socket_type): New.
            (fd_state_machine::on_socket): New.
            (fd_state_machine::check_for_socket_fd): New.
            (fd_state_machine::check_for_new_socket_fd): New.
            (fd_state_machine::on_bind): New.
            (fd_state_machine::on_listen): New.
            (fd_state_machine::on_accept): New.
            (fd_state_machine::on_connect): New.
            (fd_state_machine::can_purge_p): Don't purge socket values.
            (get_fd_state): New.
            (region_model::mark_as_valid_fd): Use get_fd_state.
            (region_model::on_socket): New.
            (region_model::on_bind): New.
            (region_model::on_listen): New.
            (region_model::on_accept): New.
            (region_model::on_connect): New.
            * sm-fd.dot: Update to reflect sm-fd.cc changes.
    
    gcc/ChangeLog:
            PR analyzer/106140
            * doc/invoke.texi (Static Analyzer Options): Add
            -Wanalyzer-fd-phase-mismatch and -Wanalyzer-fd-type-mismatch.  Add
            "socket", "bind", "listen", "accept", and "connect" to the list of
            functions known to the analyzer.
    
    gcc/testsuite/ChangeLog:
            PR analyzer/106140
            * gcc.dg/analyzer/fd-accept.c: New test.
            * gcc.dg/analyzer/fd-bind.c: New test.
            * gcc.dg/analyzer/fd-connect.c: New test.
            * gcc.dg/analyzer/fd-datagram-socket.c: New test.
            * gcc.dg/analyzer/fd-glibc-byte-stream-connection-server.c: New test.
            * gcc.dg/analyzer/fd-glibc-byte-stream-socket.c: New test.
            * gcc.dg/analyzer/fd-glibc-datagram-client.c: New test.
            * gcc.dg/analyzer/fd-glibc-datagram-socket.c: New test.
            * gcc.dg/analyzer/fd-glibc-make_named_socket.h: New test.
            * gcc.dg/analyzer/fd-listen.c: New test.
            * gcc.dg/analyzer/fd-manpage-getaddrinfo-client.c: New test.
            * gcc.dg/analyzer/fd-mappage-getaddrinfo-server.c: New test.
            * gcc.dg/analyzer/fd-socket-meaning.c: New test.
            * gcc.dg/analyzer/fd-socket-misuse.c: New test.
            * gcc.dg/analyzer/fd-stream-socket-active-open.c: New test.
            * gcc.dg/analyzer/fd-stream-socket-passive-open.c: New test.
            * gcc.dg/analyzer/fd-stream-socket.c: New test.
            * gcc.dg/analyzer/fd-symbolic-socket.c: New test.
            * gcc.dg/analyzer/pr104369-1.c: Add -Wno-analyzer-too-complex and
            -Wno-analyzer-fd-leak to options.
            * gcc.dg/analyzer/pr104369-2.c: Add -Wno-analyzer-fd-leak to
            options.
    
    Signed-off-by: David Malcolm <dmalcolm@redhat.com>
Comment 5 David Malcolm 2022-11-15 19:13:04 UTC
Implemented for GCC 13 by the above patch.