blob: ee18693c1f706378d6c64c391d577bd94c22342a [file]
.. _docs-security-false_positives:
========================
Handling false positives
========================
Agentic security scanning occasionally finds certain issues that it thinks are
security vulnerabilities but a human reviewer can tell are not. Pigweed
developers are asked to take the following steps to minimize the agents
stumbling over these false positives and creating toil.
--------------
Add assertions
--------------
First, try to update the code to include a check or assertion that constrains
certain fields or parameters to non-vulnerable values. For example, if a
certain routine is only vulnerable if a member of a class is zero, but you know
that the value is never zero, add an assertion that it isnt zero.
Typically, you can use :c:macro:`PW_DASSERT` or :c:macro:`PW_DCHECK`,
depending on if you are adding to a header or source file, respectively.
Consider the performance implications of these, as at least some projects leave
these debug assertions enabled.
Add a code comment (not a Doxygen comment) to the assertion referencing the
false positive bug report. For example:
.. code-block:: cpp
// This code is only called when `completed_` is non-zero.
// See b/8675309 for more details.
PW_DCHECK_UINT_NE(completed_, 0);
auto time_per_item = (chrono::SystemClock::now() - start) / completed_;
These should provide agents enough context to recognize the vulnerability as
unreachable.
------------------
Add security notes
------------------
Sometimes, there isnt a clear condition to assert or the cost of asserting it
would be prohibitive. In these cases, you can inform the scanner agents
directly using comments, either code comments (``//``) or Doxygen comments
(``///``).
The agents are instructed to pay special attention to any comment lines between
``@security`` and ``@endsecurity``. Doxygen has been configured to ignore these
lines. For example:
.. code-block:: cpp
/// Handles events when invoked by dispatcher-registered callbacks.
///
/// @security
///
/// This object is referenced by a callback owned by the dispatcher. It
/// appears to have a use-after-free vulnerability if the object is
/// destroyed before a callback is called, but the dispatcher is
/// notified by the destructor and ensures all related callbacks are
/// deregistered first and will never be called.
///
/// See b/8675309 for more details.
///
/// @endsecurity
As with the assertions, please include the bug number in the security note.
.. note::
There is a doc skill to help add security notes. Simply use a prompt like:
"Add a security note to MySymbol in my_module/my_file.cc for b/8675309."
--------------------
Update threat models
--------------------
If you find you have multiple cases of the same or similar false positives
within a model, you should update the modules threat model with details
identifying the pattern of false positives.
The module threat models have names like ``my_module/security.rst`` and follow
the
:cs:`threat model template <docs/sphinx/security/security.template.rst>`.