From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x102a.google.com (mail-pj1-x102a.google.com [IPv6:2607:f8b0:4864:20::102a]) by sourceware.org (Postfix) with ESMTPS id 9F4D5383088C for ; Mon, 27 Jun 2022 17:08:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9F4D5383088C Received: by mail-pj1-x102a.google.com with SMTP id c6-20020a17090abf0600b001eee794a478so2428633pjs.1 for ; Mon, 27 Jun 2022 10:08:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:from:date:message-id:subject:to; bh=AipboOOgUi2MtBv4trNonCks72bnn+kFz+PP4ZVqpoc=; b=O7AO604xmO6gHpyR7zqw9+0SWggTnnFOucSI/exXRIvyZ2MtF78up7v81PqTcKam92 hBg6VmD/AiVRoFKDkKU3k6VfrmFfm1YAGY6n/RgKqLQPWrKALDZREkVc5L/YvmW6NIeD QJ+mWj9eRgc9vxG8Ig0jI72m+i+oOfgkScfU2ir2MUIpV8m831/hm9fYVwgwEPhViJhb wxB4A82WPghS8JSou8TEhFMBqX1/6tKpSZIqmkrvOWjZD7nkEjyVYeS1kDF2ZZa1MZJM qZKJV5m6hXVChzrT/Q6L35tB7m6n2u724jjTjB0JKsJpLIEmzLqzvwIoh2iKyxnlU2vV bZ+w== X-Gm-Message-State: AJIora/xBNgKOVNyC1H9G8eSTF8ptXY6bJjp5paRlaJ+uNRAzimQvqrE 1gYLn5qtJn29HDNY/N7b+8fUTh2l39BSHd4VMD9LezoPow99EQ== X-Google-Smtp-Source: AGRyM1sCGZE1P8m5xXg8NSOt9KWdMXbY6v9p8NCFoucGDJwWO/8mIpSxgKLWaBpA7TVd1zeaE3gdU/MtUWLjAPGRQVA= X-Received: by 2002:a17:902:a515:b0:16a:3a9f:5d93 with SMTP id s21-20020a170902a51500b0016a3a9f5d93mr358638plq.139.1656349726298; Mon, 27 Jun 2022 10:08:46 -0700 (PDT) MIME-Version: 1.0 From: Mir Immad Date: Mon, 27 Jun 2022 22:38:36 +0530 Message-ID: Subject: [PATCH] analyzer PR 106003 To: gcc@gcc.gnu.org, David Malcolm X-Spam-Status: No, score=-7.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, GIT_PATCH_0, HTML_MESSAGE, KAM_NUMSUBJECT, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: gcc@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 27 Jun 2022 17:08:57 -0000 diff --git a/gcc/Makefile.in b/gcc/Makefile.in index b6dcc45a58a..04631f737ea 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -1269,6 +1269,7 @@ ANALYZER_OBJS = \ analyzer/region-model-reachability.o \ analyzer/sm.o \ analyzer/sm-file.o \ + analyzer/sm-fd.o \ analyzer/sm-malloc.o \ analyzer/sm-pattern-test.o \ analyzer/sm-sensitive.o \ diff --git a/gcc/analyzer/ChangeLog b/gcc/analyzer/ChangeLog index 53b3ffb487b..d30e94f2f62 100644 --- a/gcc/analyzer/ChangeLog +++ b/gcc/analyzer/ChangeLog @@ -1,3 +1,6 @@ +2022-06-27 Immad Mir + * sm-fd.cc: New file. + 2022-06-02 David Malcolm * checker-path.cc (checker_event::get_meaning): New. diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt index 23dfc797cea..219f6180130 100644 --- a/gcc/analyzer/analyzer.opt +++ b/gcc/analyzer/analyzer.opt @@ -66,6 +66,26 @@ 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-fd-access-mode-mismatch +Common Var(warn_analyzer_fd_mode_mismatch) Init(1) Warning +Warn about code paths in which read on write-only file descriptor is attempted, or vice versa. + +Wanalyzer-fd-double-close +Common Var(warn_analyzer_fd_double_close) Init(1) Warning +Warn about code paths in which a file descriptor can be closed more than once. + +Wanalyzer-fd-leak +Common Var(warn_analyzer_fd_leak) Init(1) Warning +Warn about code paths in which a file descriptor is not closed. + +Wanalyzer-fd-use-after-close +Common Var(warn_analyzer_fd_use_after_close) Init(1) Warning +Warn about code paths in which a read or write is performed on a closed file descriptor. + +Wanalyzer-fd-use-without-check +Common Var(warn_analyzer_fd_use_without_check) Init(1) Warning +Warn about code paths in which a file descriptor is used without being checked for validity. + 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/sm-fd.cc b/gcc/analyzer/sm-fd.cc new file mode 100644 index 00000000000..0e9833a55a8 --- /dev/null +++ b/gcc/analyzer/sm-fd.cc @@ -0,0 +1,821 @@ +/* A state machine for detecting misuses of POSIX file descriptor APIs. + Copyright (C) 2019-2022 Free Software Foundation, Inc. + Contributed by Immad Mir . + +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" +#include "system.h" +#include "coretypes.h" +#include "tree.h" +#include "function.h" +#include "basic-block.h" +#include "gimple.h" +#include "options.h" +#include "diagnostic-path.h" +#include "diagnostic-metadata.h" +#include "function.h" +#include "json.h" +#include "analyzer/analyzer.h" +#include "diagnostic-event-id.h" +#include "analyzer/analyzer-logging.h" +#include "analyzer/sm.h" +#include "analyzer/pending-diagnostic.h" +#include "analyzer/function-set.h" +#include "analyzer/analyzer-selftests.h" +#include "tristate.h" +#include "selftest.h" +#include "analyzer/call-string.h" +#include "analyzer/program-point.h" +#include "analyzer/store.h" +#include "analyzer/region-model.h" + +#include +#if ENABLE_ANALYZER + +namespace ana +{ + +namespace +{ + +/* An enum for distinguishing between three different access modes. */ + +enum access_mode +{ + READ_WRITE, + READ_ONLY, + WRITE_ONLY +}; + +/* An enum for describing the direction of an access to a file descriptor. */ + +enum access_direction +{ + DIR_READ, + DIR_WRITE +}; + +class fd_state_machine : public state_machine +{ +public: + fd_state_machine (logger *logger); + + bool + inherited_state_p () const final override + { + return false; + } + + state_machine::state_t + get_default_state (const svalue *sval) const final override + { + if (tree cst = sval->maybe_get_constant ()) + { + int val = TREE_INT_CST_LOW (cst); + if (val >= 0) + return m_constant_fd; + else + return m_invalid; + } + return m_start; + } + + bool on_stmt (sm_context *sm_ctxt, const supernode *node, + const gimple *stmt) const final override; + + void on_condition (sm_context *sm_ctxt, const supernode *node, + const gimple *stmt, const svalue *lhs, const tree_code op, + const svalue *rhs) const final override; + + bool can_purge_p (state_t s) const final override; + pending_diagnostic *on_leak (tree var) const final override; + + bool is_unchecked_fd_p (state_t s) const; + bool is_valid_fd_p (state_t s) const; + bool is_closed_fd_p (state_t s) const; + bool is_invalid_fd_p (state_t s) const; + bool is_constant_fd_p (state_t s) const; + enum access_mode get_access_mode_from_flag (int flag) const; + enum access_direction get_access_dir_from_state (state_t s) const; + + /* State for a constant file descriptor (>= 0) */ + state_t m_constant_fd; + + /* States representing a file descriptor that hasn't yet been + checked for validity after opening, for three different + access modes. */ + state_t m_unchecked_read_write; + + state_t m_unchecked_read_only; + + state_t m_unchecked_write_only; + + /* States for reperesenting a file descriptor that is known to be valid (>= + 0), for three different access modes.*/ + state_t m_valid_read_write; + + state_t m_valid_read_only; + + state_t m_valid_write_only; + + /* State for a file descriptor that is known to be invalid (< 0). */ + state_t m_invalid; + + /* State for a file descriptor that has been closed.*/ + state_t m_closed; + + /* State for a file descriptor that we do not want to track anymore . */ + state_t m_stop; + +private: + void on_open (sm_context *sm_ctxt, const supernode *node, const gimple *stmt, + const gcall *call) const; + void on_close (sm_context *sm_ctxt, const supernode *node, const gimple *stmt, + const gcall *call, const tree callee_fndecl) const; + void on_read (sm_context *sm_ctxt, const supernode *node, const gimple *stmt, + const gcall *call, const tree callee_fndecl) const; + void on_write (sm_context *sm_ctxt, const supernode *node, const gimple *stmt, + const gcall *call, const tree callee_fndecl) const; + void check_for_open_fd (sm_context *sm_ctxt, const supernode *node, + const gimple *stmt, const gcall *call, + const tree callee_fndecl, + enum access_direction access_fn) const; +}; + +/* Base diagnostic class relative to fd_state_machine. */ +class fd_diagnostic : public pending_diagnostic +{ +public: + fd_diagnostic (const fd_state_machine &sm, tree arg) : m_sm (sm), m_arg (arg) + { + } + + bool + subclass_equal_p (const pending_diagnostic &base_other) const override + { + return same_tree_p (m_arg, ((const fd_diagnostic &)base_other).m_arg); + } + + label_text + describe_state_change (const evdesc::state_change &change) override + { + if (change.m_old_state == m_sm.get_start_state () + && m_sm.is_unchecked_fd_p (change.m_new_state)) + { + if (change.m_new_state == m_sm.m_unchecked_read_write) + return change.formatted_print ("opened here as read-write"); + + if (change.m_new_state == m_sm.m_unchecked_read_only) + return change.formatted_print ("opened here as read-only"); + + if (change.m_new_state == m_sm.m_unchecked_write_only) + return change.formatted_print ("opened here as write-only"); + } + + if (change.m_new_state == m_sm.m_closed) + return change.formatted_print ("closed here"); + + if (m_sm.is_unchecked_fd_p (change.m_old_state) + && m_sm.is_valid_fd_p (change.m_new_state)) + if (change.m_expr) + return change.formatted_print ( + "assuming %qE is a valid file descriptor (>= 0)", change.m_expr); + else + return change.formatted_print ("assuming a valid file descriptor"); + + if (m_sm.is_unchecked_fd_p (change.m_old_state) + && change.m_new_state == m_sm.m_invalid) + if (change.m_expr) + return change.formatted_print ( + "assuming %qE is an invalid file descriptor (< 0)", change.m_expr); + else + return change.formatted_print ("assuming an invalid file descriptor"); + + return label_text (); + } + +protected: + const fd_state_machine &m_sm; + tree m_arg; +}; + +class fd_leak : public fd_diagnostic +{ +public: + fd_leak (const fd_state_machine &sm, tree arg) : fd_diagnostic (sm, arg) {} + + const char * + get_kind () const final override + { + return "fd_leak"; + } + + int + get_controlling_option () const final override + { + return OPT_Wanalyzer_fd_leak; + } + + bool + emit (rich_location *rich_loc) final override + { + /*CWE-775: Missing Release of File Descriptor or Handle after Effective + Lifetime + */ + diagnostic_metadata m; + m.add_cwe (775); + if (m_arg) + return warning_meta (rich_loc, m, get_controlling_option (), + "leak of file descriptor %qE", m_arg); + else + return warning_meta (rich_loc, m, get_controlling_option (), + "leak of file descriptor"); + } + + label_text + describe_state_change (const evdesc::state_change &change) final override + { + if (m_sm.is_unchecked_fd_p (change.m_new_state)) + { + m_open_event = change.m_event_id; + return label_text::borrow ("opened here"); + } + + return fd_diagnostic::describe_state_change (change); + } + + label_text + describe_final_event (const evdesc::final_event &ev) final override + { + if (m_open_event.known_p ()) + { + if (ev.m_expr) + return ev.formatted_print ("%qE leaks here; was opened at %@", + ev.m_expr, &m_open_event); + else + return ev.formatted_print ("leaks here; was opened at %@", + &m_open_event); + } + else + { + if (ev.m_expr) + return ev.formatted_print ("%qE leaks here", ev.m_expr); + else + return ev.formatted_print ("leaks here"); + } + } + +private: + diagnostic_event_id_t m_open_event; +}; + +class fd_access_mode_mismatch : public fd_diagnostic +{ +public: + fd_access_mode_mismatch (const fd_state_machine &sm, tree arg, + enum access_direction access_dir, + const tree callee_fndecl) + : m_access_dir (access_dir), m_callee_fndecl (callee_fndecl), + fd_diagnostic (sm, arg) + { + } + + const char * + get_kind () const final override + { + return "fd_access_mode_mismatch"; + } + + int + get_controlling_option () const final override + { + return OPT_Wanalyzer_fd_access_mode_mismatch; + } + + bool + emit (rich_location *rich_loc) final override + { + + switch (m_access_dir) + { + case DIR_READ: + return warning_at (rich_loc, get_controlling_option (), + "%qE on % file descriptor %qE", + m_callee_fndecl, m_arg); + case DIR_WRITE: + return warning_at (rich_loc, get_controlling_option (), + "%qE on % file descriptor %qE", + m_callee_fndecl, m_arg); + default: + gcc_unreachable (); + } + } + + bool + subclass_equal_p (const pending_diagnostic &base_other) const override + { + const fd_access_mode_mismatch &sub_other + = (const fd_access_mode_mismatch &)base_other; + return (same_tree_p (m_arg, sub_other.m_arg) + && m_callee_fndecl == sub_other.m_callee_fndecl + && m_access_dir == sub_other.m_access_dir); + } + + label_text + describe_final_event (const evdesc::final_event &ev) final override + { + switch (m_access_dir) + { + case DIR_READ: + return ev.formatted_print ("%qE on % file descriptor %qE", + m_callee_fndecl, m_arg); + case DIR_WRITE: + return ev.formatted_print ("%qE on % file descriptor %qE", + m_callee_fndecl, m_arg); + default: + gcc_unreachable (); + } + } + +private: + enum access_direction m_access_dir; + const tree m_callee_fndecl; +}; + +class double_close : public fd_diagnostic +{ +public: + double_close (const fd_state_machine &sm, tree arg) : fd_diagnostic (sm, arg) + { + } + + const char * + get_kind () const final override + { + return "double_close"; + } + + int + get_controlling_option () const final override + { + return OPT_Wanalyzer_fd_double_close; + } + bool + emit (rich_location *rich_loc) final override + { + diagnostic_metadata m; + // CWE-1341: Multiple Releases of Same Resource or Handle + m.add_cwe (1341); + return warning_meta (rich_loc, m, get_controlling_option (), + "double % of file descriptor %qE", m_arg); + } + + label_text + describe_state_change (const evdesc::state_change &change) override + { + if (m_sm.is_unchecked_fd_p (change.m_new_state)) + return label_text::borrow ("opened here"); + + if (change.m_new_state == m_sm.m_closed) + { + m_first_close_event = change.m_event_id; + return change.formatted_print ("first %qs here", "close"); + } + return fd_diagnostic::describe_state_change (change); + } + + label_text + describe_final_event (const evdesc::final_event &ev) final override + { + if (m_first_close_event.known_p ()) + return ev.formatted_print ("second %qs here; first %qs was at %@", + "close", "close", &m_first_close_event); + return ev.formatted_print ("second %qs here", "close"); + } + +private: + diagnostic_event_id_t m_first_close_event; +}; + +class fd_use_after_close : public fd_diagnostic +{ +public: + fd_use_after_close (const fd_state_machine &sm, tree arg, + const tree callee_fndecl) + : m_callee_fndecl (callee_fndecl), fd_diagnostic (sm, arg) + { + } + + const char * + get_kind () const final override + { + return "fd_use_after_close"; + } + + int + get_controlling_option () const final override + { + return OPT_Wanalyzer_fd_use_after_close; + } + + bool + emit (rich_location *rich_loc) final override + { + return warning_at (rich_loc, get_controlling_option (), + "%qE on closed file descriptor %qE", m_callee_fndecl, + m_arg); + } + + label_text + describe_state_change (const evdesc::state_change &change) override + { + if (m_sm.is_unchecked_fd_p (change.m_new_state)) + return label_text::borrow ("opened here"); + + if (change.m_new_state == m_sm.m_closed) + return change.formatted_print ("closed here"); + + return fd_diagnostic::describe_state_change (change); + } + + label_text + describe_final_event (const evdesc::final_event &ev) final override + { + return ev.formatted_print ("%qE on closed file descriptor %qE here", + m_callee_fndecl, m_arg); + } + +private: + const tree m_callee_fndecl; +}; + +class unchecked_use_of_fd : public fd_diagnostic +{ +public: + unchecked_use_of_fd (const fd_state_machine &sm, tree arg, + const tree callee_fndecl) + : m_callee_fndecl (callee_fndecl), fd_diagnostic (sm, arg) + { + } + + const char * + get_kind () const final override + { + return "unchecked_use_of_fd"; + } + + int + get_controlling_option () const final override + { + return OPT_Wanalyzer_fd_use_without_check; + } + + bool + emit (rich_location *rich_loc) final override + { + + return warning_at (rich_loc, get_controlling_option (), + "%qE on possibly invalid file descriptor %qE", + m_callee_fndecl, m_arg); + } + + bool + subclass_equal_p (const pending_diagnostic &base_other) const override + { + const unchecked_use_of_fd &sub_other + = (const unchecked_use_of_fd &)base_other; + return (same_tree_p (m_arg, sub_other.m_arg) + && m_callee_fndecl == sub_other.m_callee_fndecl); + } + + label_text + describe_state_change (const evdesc::state_change &change) override + { + if (m_sm.is_unchecked_fd_p (change.m_new_state)) + { + m_first_open_event = change.m_event_id; + return label_text::borrow ("opened here"); + } + + return fd_diagnostic::describe_state_change (change); + } + + label_text + describe_final_event (const evdesc::final_event &ev) final override + { + if (m_first_open_event.known_p ()) + return ev.formatted_print ( + "%qE could be invalid: unchecked value from %@", m_arg, + &m_first_open_event); + else + return ev.formatted_print ("%qE could be invalid", m_arg); + } + +private: + diagnostic_event_id_t m_first_open_event; + const tree m_callee_fndecl; +}; + +fd_state_machine::fd_state_machine (logger *logger) + : state_machine ("file-descriptor", logger), + m_constant_fd (add_state ("fd-constant")), + m_unchecked_read_write (add_state ("fd-unchecked-read-write")), + m_unchecked_read_only (add_state ("fd-unchecked-read-only")), + m_unchecked_write_only (add_state ("fd-unchecked-write-only")), + m_invalid (add_state ("fd-invalid")), + m_valid_read_write (add_state ("fd-valid-read-write")), + m_valid_read_only (add_state ("fd-valid-read-only")), + m_valid_write_only (add_state ("fd-valid-write-only")), + m_closed (add_state ("fd-closed")), m_stop (add_state ("fd-stop")) +{ +} + +bool +fd_state_machine::is_unchecked_fd_p (state_t s) const +{ + return (s == m_unchecked_read_write || s == m_unchecked_read_only + || s == m_unchecked_write_only); +} + +bool +fd_state_machine::is_valid_fd_p (state_t s) const +{ + return (s == m_valid_read_write || s == m_valid_read_only + || s == m_valid_write_only); +} + +enum access_mode +fd_state_machine::get_access_mode_from_flag (int flag) const +{ + /* FIXME: this code assumes the access modes on the host and + target are the same, which in practice might not be the case. */ + if (flag == O_RDONLY) + { + return READ_ONLY; + } + else if (flag == O_WRONLY) + { + return WRITE_ONLY; + } + return READ_WRITE; +} + +enum access_direction +fd_state_machine::get_access_dir_from_state (state_t state) const +{ + if (state == m_unchecked_read_only || state == m_valid_read_only) + return DIR_READ; + else if (state == m_unchecked_write_only || state == m_valid_write_only) + return DIR_WRITE; +} + +bool +fd_state_machine::is_closed_fd_p (state_t state) const +{ + return (state == m_closed); +} + +bool +fd_state_machine::is_invalid_fd_p (state_t state) const +{ + return (state == m_invalid); +} + +bool +fd_state_machine::is_constant_fd_p (state_t state) const +{ + return (state == m_constant_fd); +} + +bool +fd_state_machine::on_stmt (sm_context *sm_ctxt, const supernode *node, + const gimple *stmt) const +{ + + if (const gcall *call = dyn_cast (stmt)) + if (tree callee_fndecl = sm_ctxt->get_fndecl_for_call (call)) + { + if (is_named_call_p (callee_fndecl, "open", call, 2)) + { + on_open (sm_ctxt, node, stmt, call); + return true; + } // "open" + + if (is_named_call_p (callee_fndecl, "close", call, 1)) + { + on_close (sm_ctxt, node, stmt, call, callee_fndecl); + return true; + } // "close" + + if (is_named_call_p (callee_fndecl, "write", call, 3)) + { + on_write (sm_ctxt, node, stmt, call, callee_fndecl); + return true; + } // "write" + + if (is_named_call_p (callee_fndecl, "read", call, 3)) + { + on_read (sm_ctxt, node, stmt, call, callee_fndecl); + return true; + } // "read" + } + + return false; +} + +void +fd_state_machine::on_open (sm_context *sm_ctxt, const supernode *node, + const gimple *stmt, const gcall *call) const +{ + tree lhs = gimple_call_lhs (call); + if (lhs) + { + tree arg = gimple_call_arg (call, 1); + if (TREE_CODE (arg) == INTEGER_CST) + { + int flag = TREE_INT_CST_LOW (arg); + enum access_mode mode = get_access_mode_from_flag (flag); + + switch (mode) + { + case READ_ONLY: + sm_ctxt->on_transition (node, stmt, lhs, m_start, + m_unchecked_read_only); + break; + case WRITE_ONLY: + sm_ctxt->on_transition (node, stmt, lhs, m_start, + m_unchecked_write_only); + break; + default: + sm_ctxt->on_transition (node, stmt, lhs, m_start, + m_unchecked_read_write); + } + } + } + else + { + sm_ctxt->warn (node, stmt, NULL_TREE, new fd_leak (*this, NULL_TREE)); + } +} + +void +fd_state_machine::on_close (sm_context *sm_ctxt, const supernode *node, + const gimple *stmt, const gcall *call, + const tree callee_fndecl) const +{ + tree arg = gimple_call_arg (call, 0); + state_t state = sm_ctxt->get_state (stmt, arg); + tree diag_arg = sm_ctxt->get_diagnostic_tree (arg); + + sm_ctxt->on_transition (node, stmt, arg, m_start, m_closed); + sm_ctxt->on_transition (node, stmt, arg, m_unchecked_read_write, m_closed); + sm_ctxt->on_transition (node, stmt, arg, m_unchecked_read_only, m_closed); + sm_ctxt->on_transition (node, stmt, arg, m_unchecked_write_only, m_closed); + sm_ctxt->on_transition (node, stmt, arg, m_valid_read_write, m_closed); + sm_ctxt->on_transition (node, stmt, arg, m_valid_read_only, m_closed); + sm_ctxt->on_transition (node, stmt, arg, m_valid_write_only, m_closed); + sm_ctxt->on_transition (node, stmt, arg, m_constant_fd, m_closed); + + if (is_closed_fd_p (state)) + { + + sm_ctxt->warn (node, stmt, arg, new double_close (*this, diag_arg)); + sm_ctxt->set_next_state (stmt, arg, m_stop); + } +} +void +fd_state_machine::on_read (sm_context *sm_ctxt, const supernode *node, + const gimple *stmt, const gcall *call, + const tree callee_fndecl) const +{ + check_for_open_fd (sm_ctxt, node, stmt, call, callee_fndecl, DIR_READ); +} +void +fd_state_machine::on_write (sm_context *sm_ctxt, const supernode *node, + const gimple *stmt, const gcall *call, + const tree callee_fndecl) const +{ + check_for_open_fd (sm_ctxt, node, stmt, call, callee_fndecl, DIR_WRITE); +} + +void +fd_state_machine::check_for_open_fd (sm_context *sm_ctxt, const supernode *node, + const gimple *stmt, const gcall *call, + const tree callee_fndecl, + enum access_direction access_fn) const +{ + tree arg = gimple_call_arg (call, 0); + tree diag_arg = sm_ctxt->get_diagnostic_tree (arg); + state_t state = sm_ctxt->get_state (stmt, arg); + enum access_direction access_dir = get_access_dir_from_state (state); + + if (is_closed_fd_p (state)) + { + sm_ctxt->warn (node, stmt, arg, + new fd_use_after_close (*this, diag_arg, callee_fndecl)); + } + + else + { + + if (!is_valid_fd_p (state)) + { + if (!is_constant_fd_p (state)) + sm_ctxt->warn ( + node, stmt, arg, + new unchecked_use_of_fd (*this, diag_arg, callee_fndecl)); + } + switch (access_fn) + { + case DIR_READ: + if (access_dir == DIR_WRITE) + { + tree diag_arg = sm_ctxt->get_diagnostic_tree (arg); + sm_ctxt->warn (node, stmt, arg, + new fd_access_mode_mismatch ( + *this, diag_arg, access_dir, callee_fndecl)); + } + + break; + case DIR_WRITE: + + if (access_dir == DIR_READ) + { + tree diag_arg = sm_ctxt->get_diagnostic_tree (arg); + sm_ctxt->warn (node, stmt, arg, + new fd_access_mode_mismatch ( + *this, diag_arg, access_dir, callee_fndecl)); + } + break; + } + } +} + +void +fd_state_machine::on_condition (sm_context *sm_ctxt, const supernode *node, + const gimple *stmt, const svalue *lhs, + enum tree_code op, const svalue *rhs) const +{ + + if (!rhs->all_zeroes_p ()) + return; + + if (op == GE_EXPR) + { + sm_ctxt->on_transition (node, stmt, lhs, m_unchecked_read_write, + m_valid_read_write); + sm_ctxt->on_transition (node, stmt, lhs, m_unchecked_read_only, + m_valid_read_only); + sm_ctxt->on_transition (node, stmt, lhs, m_unchecked_write_only, + m_valid_write_only); + } + else if (op == LT_EXPR) + { + sm_ctxt->on_transition (node, stmt, lhs, m_unchecked_read_write, + m_invalid); + sm_ctxt->on_transition (node, stmt, lhs, m_unchecked_read_only, + m_invalid); + sm_ctxt->on_transition (node, stmt, lhs, m_unchecked_write_only, + m_invalid); + } +} + +bool +fd_state_machine::can_purge_p (state_t s) const +{ + if (is_unchecked_fd_p (s) || is_valid_fd_p (s)) + return false; + else + return true; +} + +pending_diagnostic * +fd_state_machine::on_leak (tree var) const +{ + return new fd_leak (*this, var); +} +} // anonymus namespace + +state_machine * +make_fd_state_machine (logger *logger) +{ + return new fd_state_machine (logger); +} +} // namespace ana + +#endif // ENABLE_ANALYZER \ No newline at end of file diff --git a/gcc/analyzer/sm.cc b/gcc/analyzer/sm.cc index 622cb0b7ab3..24c20b894cd 100644 --- a/gcc/analyzer/sm.cc +++ b/gcc/analyzer/sm.cc @@ -167,6 +167,7 @@ make_checkers (auto_delete_vec &out, logger *logger) { out.safe_push (make_malloc_state_machine (logger)); out.safe_push (make_fileptr_state_machine (logger)); + out.safe_push (make_fd_state_machine (logger)); /* The "taint" checker must be explicitly enabled (as it currently leads to state explosions that stop the other checkers working). */ if (flag_analyzer_checker) diff --git a/gcc/analyzer/sm.h b/gcc/analyzer/sm.h index 4cc54531c56..e80ef1fac37 100644 --- a/gcc/analyzer/sm.h +++ b/gcc/analyzer/sm.h @@ -301,6 +301,7 @@ extern state_machine *make_sensitive_state_machine (logger *logger); extern state_machine *make_signal_state_machine (logger *logger); extern state_machine *make_pattern_test_state_machine (logger *logger); extern state_machine *make_va_list_state_machine (logger *logger); +extern state_machine *make_fd_state_machine (logger *logger); } // namespace ana diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 174bc09e5cf..b4a46dddfbb 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -9709,6 +9709,11 @@ Enabling this option effectively enables the following warnings: -Wanalyzer-double-fclose @gol -Wanalyzer-double-free @gol -Wanalyzer-exposure-through-output-file @gol +-Wanalyzer-fd-access-mode-mismatch @gol +-Wanalyzer-fd-double-close @gol +-Wanalyzer-fd-leak @gol +-Wanalyzer-fd-use-after-close @gol +-Wanalyzer-fd-use-without-check @gol -Wanalyzer-file-leak @gol -Wanalyzer-free-of-non-heap @gol -Wanalyzer-malloc-leak @gol @@ -9783,6 +9788,56 @@ 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 -Wno-analyzer-fd-access-mode-mismatch +@opindex Wanalyzer-fd-access-mode-mismatch +@opindex Wno-analyzer-fd-access-mode-mismatch +This warning requires @option{-fanalyzer}, which enables it; use +@option{-Wno-analyzer-fd-access-mode-mismatch} +to disable it. + +This diagnostic warns for paths through code in which a +@code{read} on a write-only file descriptor is attempted, or vice versa + +@item -Wno-analyzer-fd-double-close +@opindex Wanalyzer-fd-double-close +@opindex Wno-analyzer-fd-double-close +This warning requires @option{-fanalyzer}, which enables it; use +@option{-Wno-analyzer-fd-double-close} +to disable it. + +This diagnostic warns for paths through code in which a +file descriptor can be closed more than once. + +@item -Wno-analyzer-fd-leak +@opindex Wanalyzer-fd-leak +@opindex Wno-analyzer-fd-leak +This warning requires @option{-fanalyzer}, which enables it; use +@option{-Wno-analyzer-fd-leak} +to disable it. + +This diagnostic warns for paths through code in which an +open file descriptor is leaked. + +@item -Wno-analyzer-fd-use-after-close +@opindex Wanalyzer-fd-use-after-close +@opindex Wno-analyzer-fd-use-after-close +This warning requires @option{-fanalyzer}, which enables it; use +@option{-Wno-analyzer-fd-use-after-close} +to disable it. + +This diagnostic warns for paths through code in which a +read or write is called on a closed file descriptor. + +@item -Wno-analyzer-fd-use-without-check +@opindex Wanalyzer-fd-use-without-check +@opindex Wno-analyzer-fd-use-without-check +This warning requires @option{-fanalyzer}, which enables it; use +@option{-Wno-analyzer-fd-use-without-check} +to disable it. + +This diagnostic warns for paths through code in which a +file descriptor is used without being checked for validity. + @item -Wno-analyzer-file-leak @opindex Wanalyzer-file-leak @opindex Wno-analyzer-file-leak diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 604f4e3b0a8..37173529b2d 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2022-06-27 Immad Mir + * gcc.dg/analyzer/fd-1.c: New test. + * gcc.dg/analyzer/fd-2.c: New test. + * gcc.dg/analyzer/fd-3.c: New test. + * gcc.dg/analyzer/fd-4.c: New test. + 2022-06-10 Nathan Sidwell * g++.dg/modules/init-3_a.C: New. diff --git a/gcc/testsuite/gcc.dg/analyzer/fd-1.c b/gcc/testsuite/gcc.dg/analyzer/fd-1.c new file mode 100644 index 00000000000..aaff8054212 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/fd-1.c @@ -0,0 +1,32 @@ +int open(const char *, int mode); +#define O_RDONLY 0 +#define O_WRONLY 1 +#define O_RDWR 2 + +void +test_1 (const char *path) +{ + int fd = open (path, O_RDONLY); /* { dg-message "\\(1\\) opened here" } */ + return; /* { dg-warning "leak of file descriptor 'fd' \\\[CWE-775\\\]" "warning" } */ + /* { dg-message "\\(2\\) 'fd' leaks here; was opened at \\(1\\)" "event" { target *-*-* } .-1 } */ +} + +void +test_2 (const char *path) +{ + int fd = open (path, O_RDWR); /* { dg-message "\\(1\\) opened here" } */ + if (fd >= 0) /* { dg-message "\\(2\\) assuming 'fd' is a valid file descriptor" "event1" } */ + /* { dg-message "\\(3\\) following 'true' branch \\(when 'fd >= 0'\\)..." "event2" { target *-*-* } .-1 } */ + { + return; /* { dg-warning "leak of file descriptor 'fd' \\\[CWE-775\\\]" "warning" } */ + /* { dg-message "\\(4\\) ...to here" "event1" { target *-*-* } .-1 } */ + /* { dg-message "\\(5\\) 'fd' leaks here; was opened at \\(1\\)" "event2" { target *-*-* } .-2 } */ + } +} + +void test_3 (const char *path) +{ + open(path, O_RDONLY); /* { dg-warning "leak of file descriptor \\\[CWE-775\\\]" } */ + /* { dg-message "\\(1\\) leaks here" "" { target *-*-* } .-1 } */ +} + diff --git a/gcc/testsuite/gcc.dg/analyzer/fd-2.c b/gcc/testsuite/gcc.dg/analyzer/fd-2.c new file mode 100644 index 00000000000..96ccf2f7ba8 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/fd-2.c @@ -0,0 +1,49 @@ +int open(const char *, int mode); +void close(int fd); +#define O_RDONLY 0 +#define O_WRONLY 1 +#define O_RDWR 2 +#define STDIN 0 + +void +test_1 (const char *path) +{ + int fd = open (path, O_RDWR); /* { dg-message "\\(1\\) opened here" } */ + close (fd); /* { dg-message "\\(2\\) first 'close' here" "event1" } */ + close (fd); /* { dg-warning "double 'close' of file descriptor 'fd' \\\[CWE-1341\\\]" "warning" } */ + /* { dg-message "\\(3\\) second 'close' here; first 'close' was at \\(2\\)" "event2" { target *-*-* } .-1 } */ +} + +void +test_2 (const char *path) +{ + int fd = open (path, O_RDWR); /* { dg-message "\\(1\\) opened here" } */ + if (fd < 0) /* { dg-message "\\(2\\) assuming 'fd' is a valid file descriptor \\(>= 0\\)" "event1" } */ + /* { dg-message "\\(3\\) following 'false' branch \\(when 'fd >= 0'\\)..." "event2" { target *-*-* } .-1 } */ + return; + close (fd); /* { dg-message "\\(4\\) ...to here" "event1" } */ + /* { dg-message "\\(5\\) first 'close' here" "event2" { target *-*-* } .-1 } */ + close (fd); /* { dg-warning "double 'close' of file descriptor 'fd' \\\[CWE-1341\\\]" "warning" } */ + /* {dg-message "\\(6\\) second 'close' here; first was at \\(5\\)" "" { target *-*-* } .-1 } */ +} + +void +test_3 () +{ + /* FD 0 is stdin at the entry to "main" and thus read-only, but we have no + guarantees here that it hasn't been closed and then reopened for + writing, so we can't issue a warning */ + + int fd = STDIN; + close(fd); /* { dg-message "\\(1\\) first 'close' here" } */ + close(fd); /* { dg-warning "double 'close' of file descriptor 'fd' \\\[CWE-1341\\\]" "warning" } */ + /* { dg-message "\\(2\\) second 'close' here; first 'close' was at \\(1\\)" "event2" { target *-*-* } .-1 } */ +} + +void +test_4 () +{ + int fd = -1; + close(fd); + close(fd); +} \ No newline at end of file diff --git a/gcc/testsuite/gcc.dg/analyzer/fd-3.c b/gcc/testsuite/gcc.dg/analyzer/fd-3.c new file mode 100644 index 00000000000..22d29389ff9 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/fd-3.c @@ -0,0 +1,57 @@ +int open(const char *, int mode); +void close(int fd); +int write (int fd, void *buf, int nbytes); +int read (int fd, void *buf, int nbytes); +int some_condition(); + +#define O_RDONLY 0 +#define O_WRONLY 1 +#define O_RDWR 2 +#define STDIN 0 +#define O_NOATIME 262144 + +void +test_1 (const char *path, void *buf) +{ + int fd = open (path, O_RDWR); /* { dg-message "\\(1\\) opened here" } */ + write (fd, buf, 1); /* { dg-message "\\(2\\) 'fd' could be invalid: unchecked value from \\(1\\)" } */ + /* { dg-warning "'write' on possibly invalid file descriptor 'fd'" "warning" { target *-*-* } .-1 } */ + close(fd); +} + +void +test_2 (const char *path, void *buf) +{ + int fd = open (path, O_RDWR); /* { dg-message "\\(1\\) opened here" } */ + read (fd, buf, 1); /* { dg-message "\\(2\\) 'fd' could be invalid: unchecked value from \\(1\\)" } */ + /* { dg-warning "'read' on possibly invalid file descriptor 'fd'" "warning" { target *-*-* } .-1 } */ + close (fd); +} + +void +test_3 (void *buf) +{ + int fd = -1; + read (fd, buf, 1); /* { dg-warning "'read' on possibly invalid file descriptor 'fd'" } */ + /* { dg-message "\\(1\\) 'fd' could be invalid" "" { target *-*-* } .-1 } */ +} + +void +test_4 (void *buf) +{ + int fd = STDIN; + read (fd, buf, 1); + close(fd); +} + +void +test_5 (char *path, void *buf) +{ + int flags = O_RDONLY; + if (some_condition()) + flags |= O_NOATIME; + int fd = open (path, flags); + read(fd, buf, 1); /* { dg-warning "'read' on possibly invalid file descriptor 'fd'" } */ + /* { dg-message "\\(1\\) 'fd' could be invalid" "" { target *-*-* } .-1 } */ + close(fd); +} diff --git a/gcc/testsuite/gcc.dg/analyzer/fd-4.c b/gcc/testsuite/gcc.dg/analyzer/fd-4.c new file mode 100644 index 00000000000..56828765bdd --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/fd-4.c @@ -0,0 +1,60 @@ +int open(const char *, int mode); +void close(int fd); +int write (int fd, void *buf, int nbytes); +int read (int fd, void *buf, int nbytes); + +#define O_RDONLY 0 +#define O_WRONLY 1 +#define O_RDWR 2 + + +void +test_1 (const char *path, void *buf) +{ + int fd = open (path, O_RDONLY); /* { dg-message "opened here as read-only" } */ + if (fd >= 0) /* { dg-message "assuming 'fd' is a valid file descriptor \\(>= 0\\)" "event1" } */ + /* { dg-message "following 'true' branch \\(when 'fd >= 0'\\)..." "event2" { target *-*-* } .-1 } */ + { + write (fd, buf, 1); /* { dg-warning "'write' on 'read-only' file descriptor 'fd'" "warning" } */ + /* { dg-message "\\(4\\) ...to here" "event1" { target *-*-* } .-1 } */ + /* { dg-message "\\(5\\) 'write' on 'read-only' file descriptor 'fd'" "event2" { target *-*-* } .-2 } */ + close (fd); + } +} + +void +test_2 (const char *path, void *buf) +{ + int fd = open (path, O_WRONLY); /* { dg-message "opened here as write-only" } */ + if (fd >= 0) /* { dg-message "assuming 'fd' is a valid file descriptor \\(>= 0\\)" "event1" } */ + /* { dg-message "following 'true' branch \\(when 'fd >= 0'\\)..." "event2" { target *-*-* } .-1 } */ + { + read (fd, buf, 1); /* { dg-warning "'read' on 'write-only' file descriptor 'fd'" "warning" } */ + /* { dg-message "\\(4\\) ...to here" "event1" { target *-*-* } .-1 } */ + /* { dg-message "\\(5\\) 'read' on 'write-only' file descriptor 'fd'" "event2" { target *-*-* } .-2 } */ + close (fd); + } +} + + +void test_4 (const char *path, void *buf) +{ + int fd = open (path, O_RDWR); /* { dg-message "\\(1\\) opened here" } */ + if (fd >= 0) + { + close(fd); /* {dg-message "\\(2\\) closed here"} */ + read(fd, buf, 1); /* { dg-warning "'read' on closed file descriptor 'fd'" } */ + /* {dg-message "\\(3\\) 'read' on closed file descriptor 'fd' here" "" {target *-*-*} .-1 } */ + } +} + +void test_5 (const char *path, void *buf) +{ + int fd = open (path, O_RDWR); /* { dg-message "\\(1\\) opened here" } */ + if (fd >= 0) + { + close(fd); /* {dg-message "\\(2\\) closed here"} */ + write(fd, buf, 1); /* { dg-warning "'write' on closed file descriptor 'fd'" } */ + /* {dg-message "\\(3\\) 'write' on closed file descriptor 'fd' here" "" {target *-*-*} .-1 } */ + } +} \ No newline at end of file