Bug 80895 - format not a string literal and no format arguments; in GCC itself
Summary: format not a string literal and no format arguments; in GCC itself
Status: RESOLVED INVALID
Alias: None
Product: gcc
Classification: Unclassified
Component: other (show other bugs)
Version: unknown
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-05-27 00:47 UTC by Andrew Miloradovsky
Modified: 2017-05-27 02:12 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
a trivial patch (2.96 KB, patch)
2017-05-27 00:47 UTC, Andrew Miloradovsky
Details | Diff
excluding few cases (2.70 KB, patch)
2017-05-27 01:15 UTC, Andrew Miloradovsky
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Miloradovsky 2017-05-27 00:47:07 UTC
Created attachment 41428 [details]
a trivial patch

GCC couldn't be compiled with `-Werror=format-security`, now it can.
The motivation is here (although that post is somewhat outdated):

- https://unix.stackexchange.com/questions/356232/disabling-the-security-hardening-options-for-a-nix-shell-environment
Comment 1 Andrew Pinski 2017-05-27 00:59:31 UTC
This patch is incorrect:
   /* The gmsgid may be a format string with %< and %>. */
-  warned = pedwarn (exploc, opt, gmsgid);
+  warned = pedwarn (exploc, opt, "%s", gmsgid);

See that comment of why.
Comment 2 Andrew Miloradovsky 2017-05-27 01:15:49 UTC
Created attachment 41429 [details]
excluding few cases

where it's said explicitly that `msgid` may be a format string
Comment 3 Andrew Pinski 2017-05-27 01:19:37 UTC
Still broken:
   else
-    error (gmsgid);
+    error ("%s", gmsgid);

And more where you have *msgid.

I bet invalid_func_diag has similar too.
Comment 4 Andrew Miloradovsky 2017-05-27 01:24:58 UTC
Hmm, man printf(3) doesn't tell anything about the formats, what are they for?
Comment 5 Andrew Miloradovsky 2017-05-27 01:26:59 UTC
After all I might simply insert that comment everywhere, to reduce confusion.
Comment 6 Andrew Pinski 2017-05-27 01:30:36 UTC
(In reply to Andrew Miloradovsky from comment #4)
> Hmm, man printf(3) doesn't tell anything about the formats, what are they
> for?

They are part of the gcc diagnostic format. Gcc does not use printf directly for those functions.  See how they have the gcc_format attribute rather than the printf_format attribute.
Comment 7 Martin Sebor 2017-05-27 01:45:33 UTC
GCC format strings can contain directives like %< and %> that trigger special handling (such as quoting and highlighting).  For example, in gcc/c-family/c-common.c:

    message = catenate_messages (gmsgid, " before %<#pragma%>");
    ...
  if (message)
    {
      error (message);
      free (message);

error message ends up quoting #pragma and displaying it in the color set by quote= in GCC_COLORS:

https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Message-Formatting-Options.html

To do what you suggest I think GCC would need to recognize and use a special directive telling to treat its argument as a format string (i.e., implement at least a limited form recursive format string handling).
Comment 8 Andrew Miloradovsky 2017-05-27 02:12:31 UTC
Well, then it seems like

- the entire patch is wrong, and all those changes shouldn't be made
- inserting the comment before every diagnostic output call is not worth it
- in future, this mechanism may be changed, to filter out the warnings
- but, ATM, format-security should always be disabled when compiling GCC
- so the "bug" may be closed now; at least I learned something new; thanks