RFC Report modes for unsafe function reporting (original) (raw)

Motivation

The security.insecureAPI.DeprecatedOrUnsafeBufferHandling checker currently reports warnings for deprecated buffer handling functions (e.g., memcpy, memset, memmove, strcpy, strcat) only when compiling with C11 standard. However, there are two distinct use cases that require different behaviors:

Use Case 1: Vulnerability Detection

Security auditors and vulnerability scanners need to identify all potentially unsafe function usages, regardless of the C standard version or Annex K availability. These functions are problematic even in C99 and earlier standards, and the checker should flag them for security review purposes.

Use Case 2: Actionable Warnings

Developers want to receive warnings only when they can actually act on them. Since the checker suggests using _s-suffixed alternatives (e.g., memcpy_s, strcpy_s) from C11 Annex K, warnings are only actionable when:

1. C11 standard is enabled, AND
2. Annex K is available (indicated by __STDC_LIB_EXT1__ and __STDC_WANT_LIB_EXT1__ macros)

Currently, the checker suggests _s alternatives even when Annex K may not be available, leading to non-actionable warnings.

Current State

There is an ongoing PR (#168704) that adds a ReportInC99AndEarlier boolean flag to enable reporting in non-C11 code. However, this doesn’t address the Annex K availability concern raised in the PR discussion.

The current implementation:

• Reports warnings when C11 is enabled (regardless of Annex K availability)
• Always suggests _s alternatives, even when Annex K may not be available
• Does not detect Annex K availability

Proposed Solutions

Alternative A: Single ReportMode Flag

Add a string option ReportMode with three values:

all (Vulnerability Detection Mode)

• Reports all unsafe functions regardless of C11 or Annex K availability
• Suggests _s alternatives when Annex K is available
• Suggests generic alternatives (e.g., “use bounds-checking function”) when Annex K is not available
• Use case: Security auditing, vulnerability scanning

actionable (Actionable Warnings Mode)

• Only reports when Annex K is available and enabled
• Always suggests specific _s alternatives (since Annex K is guaranteed)
• Use case: Development workflow where developers only want fixable warnings

c11-only (Backward Compatible - Default)

• Reports when C11 is enabled (current behavior)
• Always suggests _s alternatives
• Maintains backward compatibility
• Note that we may not want this backward compatibility, this is up for discussion

Pros:

• Single decision point, simpler mental model
• Clear semantics for each mode
• Easy to document and understand
• Covers both use cases

Cons:

• Requires detecting Annex K availability (via __STDC_LIB_EXT1__ and __STDC_WANT_LIB_EXT1__ macros) (is this a real con?)

Alternative B: Two Boolean Flags

Add two separate boolean flags for granular control:

1. ReportAllUnsafeFunctions (default: false)
   • If true: Report all unsafe functions regardless of C11/Annex K
   • If false: Only report when C11 enabled (current behavior)
2. RequireAnnexKForSuggestions (default: false)
   • If true: Only suggest _s alternatives when Annex K is available
   • If false: Always suggest _s alternatives (current behavior)

Combination behaviors:

• ReportAllUnsafeFunctions=false, RequireAnnexKForSuggestions=false: Current behavior
• ReportAllUnsafeFunctions=true, RequireAnnexKForSuggestions=false: Report all, always suggest _s
• ReportAllUnsafeFunctions=false, RequireAnnexKForSuggestions=true: Report when C11, only suggest _s when Annex K available
• ReportAllUnsafeFunctions=true, RequireAnnexKForSuggestions=true: Report all, only suggest _s when Annex K available

Pros:

• More granular control
• Can be combined for different behaviors
• Each flag has a single, clear purpose

Cons:

• More complex mental model (4 combinations)
• Harder to document all combinations
• May be confusing which combination to use

Alternative C: Annex K Detection-Based Default Behavior

Change the default behavior to detect Annex K availability rather than just checking for C11:

• Default behavior: Report warnings when Annex K is available (detected via __STDC_LIB_EXT1__ and __STDC_WANT_LIB_EXT1__ macros)
• Add ReportAllUnsafeFunctions flag to enable vulnerability detection mode (reports all unsafe functions regardless of Annex K)
• Always suggest _s alternatives (since we only report when Annex K is available by default)

Pros:

• Makes warnings actionable by default
• Simpler flag structure (only one flag needed for vulnerability mode)
• Addresses the concern raised in PR #168704

Cons:

• Breaking change (different default behavior)
• May surprise users expecting current behavior
• Doesn’t provide a way to get current behavior (C11-only without Annex K check)

Annex K Detection

Annex K availability can be detected by checking:

1. C11 standard is enabled
2. __STDC_LIB_EXT1__ macro is defined (indicates library support)
3. __STDC_WANT_LIB_EXT1__ macro is defined and equals 1 (indicates user opt-in)

This detection logic already exists in clang-tidy’s UnsafeFunctionsCheck and can be reused.

Recommendation

Alternative A (Single ReportMode Flag) is recommended because:

1. It provides a clear, intuitive interface for both use cases
2. It makes the checker’s behavior explicit and easy to understand
3. It addresses both the vulnerability detection and actionable warnings use cases
4. Since PR #168704 is not yet merged, we can replace ReportInC99AndEarlier with ReportMode without compatibility concerns

Note on Alternatives:

• Alternative A vs B: Choose between single flag (simpler) vs two flags (more granular)
• Alternative C: Consider if we want to make warnings actionable by default (breaking change)

Questions for Discussion

1. Should we replace ReportInC99AndEarlier from PR #168704 with ReportMode (or another alternative), or extend the existing flag? Since the PR is not merged, we have full freedom to choose the best approach.
2. In all mode, when Annex K is not available, what should we suggest? Generic text, platform-specific alternatives (e.g., strlcpy on BSD), or just note the issue?
3. Should Annex K detection be a prerequisite for any of these modes, or should it be optional?
4. Which alternative (A, B, or C) best serves the community’s needs?
5. Is there better alternative than the ones proposed?

Related Work

• Ongoing PR: #168704 - Adds ReportInC99AndEarlier flag
• clang-tidy’s bugprone-unsafe-functions check already implements Annex K detection
• Discussion in PR #168704 about making warnings actionable based on Annex K availability