* [PATCH] config.mak.dev: add -Wformat-security
@ 2018-09-07 18:19 Jeff King
2018-09-08 13:38 ` Duy Nguyen
0 siblings, 1 reply; 3+ messages in thread
From: Jeff King @ 2018-09-07 18:19 UTC (permalink / raw)
To: git
We currently build cleanly with -Wformat-security, and it's
a good idea to make sure we continue to do so (since calls
that trigger the warning may be security vulnerabilities).
Note that we cannot use the stronger -Wformat-nonliteral, as
there are case where we are clever with passing around
pointers to string literals. E.g., bisect_rev_setup() takes
bad_format and good_format parameters. These ultimately come
from literals, but they still trigger the warning.
Some of these might be fixable (e.g., by passing flags from
which we locally select a format), and might even be worth
fixing (not because of security, but just because it's an
easy mistake to pass the wrong format). But there are other
cases which are likely quite hard to fix (we actually
generate formats in a local buffer in some cases). So let's
punt on that for now and start with -Wformat-security, which
is supposed to catch the most important cases.
Signed-off-by: Jeff King <peff@peff.net>
---
config.mak.dev | 1 +
1 file changed, 1 insertion(+)
diff --git a/config.mak.dev b/config.mak.dev
index 9a998149d9..f832752454 100644
--- a/config.mak.dev
+++ b/config.mak.dev
@@ -14,6 +14,7 @@ CFLAGS += -Wpointer-arith
CFLAGS += -Wstrict-prototypes
CFLAGS += -Wunused
CFLAGS += -Wvla
+CFLAGS += -Wformat-security
ifndef COMPILER_FEATURES
COMPILER_FEATURES := $(shell ./detect-compiler $(CC))
--
2.19.0.rc2.594.gc4806cd463
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] config.mak.dev: add -Wformat-security
2018-09-07 18:19 [PATCH] config.mak.dev: add -Wformat-security Jeff King
@ 2018-09-08 13:38 ` Duy Nguyen
2018-09-08 16:23 ` [PATCH v2] " Jeff King
0 siblings, 1 reply; 3+ messages in thread
From: Duy Nguyen @ 2018-09-08 13:38 UTC (permalink / raw)
To: Jeff King; +Cc: Git Mailing List
On Fri, Sep 7, 2018 at 8:21 PM Jeff King <peff@peff.net> wrote:
>
> We currently build cleanly with -Wformat-security, and it's
> a good idea to make sure we continue to do so (since calls
> that trigger the warning may be security vulnerabilities).
Nice. I had this flag in my config.mak too before switching to
DEVELOPER=1. Didn't realize I lost the flag until now.
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> config.mak.dev | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/config.mak.dev b/config.mak.dev
> index 9a998149d9..f832752454 100644
> --- a/config.mak.dev
> +++ b/config.mak.dev
> @@ -14,6 +14,7 @@ CFLAGS += -Wpointer-arith
> CFLAGS += -Wstrict-prototypes
> CFLAGS += -Wunused
> CFLAGS += -Wvla
> +CFLAGS += -Wformat-security
Maybe keep it sorted
--
Duy
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v2] config.mak.dev: add -Wformat-security
2018-09-08 13:38 ` Duy Nguyen
@ 2018-09-08 16:23 ` Jeff King
0 siblings, 0 replies; 3+ messages in thread
From: Jeff King @ 2018-09-08 16:23 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List
On Sat, Sep 08, 2018 at 03:38:19PM +0200, Duy Nguyen wrote:
> On Fri, Sep 7, 2018 at 8:21 PM Jeff King <peff@peff.net> wrote:
> >
> > We currently build cleanly with -Wformat-security, and it's
> > a good idea to make sure we continue to do so (since calls
> > that trigger the warning may be security vulnerabilities).
>
> Nice. I had this flag in my config.mak too before switching to
> DEVELOPER=1. Didn't realize I lost the flag until now.
I thought I used to use it, too, but I realized recently that I never
did (I experimented with format-nonliteral a long time ago but gave up).
AFAIK we've never actually had a case that this triggered, because it's
so narrow (as opposed to format-nonliteral).
> > diff --git a/config.mak.dev b/config.mak.dev
> > index 9a998149d9..f832752454 100644
> > --- a/config.mak.dev
> > +++ b/config.mak.dev
> > @@ -14,6 +14,7 @@ CFLAGS += -Wpointer-arith
> > CFLAGS += -Wstrict-prototypes
> > CFLAGS += -Wunused
> > CFLAGS += -Wvla
> > +CFLAGS += -Wformat-security
>
> Maybe keep it sorted
Heh, I didn't even notice the original was sorted. Here it is in the
right spot:
-- >8 --
Subject: config.mak.dev: add -Wformat-security
We currently build cleanly with -Wformat-security, and it's
a good idea to make sure we continue to do so (since calls
that trigger the warning may be security vulnerabilities).
Note that we cannot use the stronger -Wformat-nonliteral, as
there are case where we are clever with passing around
pointers to string literals. E.g., bisect_rev_setup() takes
bad_format and good_format parameters. These ultimately come
from literals, but they still trigger the warning.
Some of these might be fixable (e.g., by passing flags from
which we locally select a format), and might even be worth
fixing (not because of security, but just because it's an
easy mistake to pass the wrong format). But there are other
cases which are likely quite hard to fix (we actually
generate formats in a local buffer in some cases). So let's
punt on that for now and start with -Wformat-security, which
is supposed to catch the most important cases.
Signed-off-by: Jeff King <peff@peff.net>
---
config.mak.dev | 1 +
1 file changed, 1 insertion(+)
diff --git a/config.mak.dev b/config.mak.dev
index 9a998149d9..92d268137f 100644
--- a/config.mak.dev
+++ b/config.mak.dev
@@ -7,6 +7,7 @@ CFLAGS += -pedantic
CFLAGS += -DUSE_PARENS_AROUND_GETTEXT_N=0
endif
CFLAGS += -Wdeclaration-after-statement
+CFLAGS += -Wformat-security
CFLAGS += -Wno-format-zero-length
CFLAGS += -Wold-style-definition
CFLAGS += -Woverflow
--
2.19.0.rc2.594.gc4806cd463
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-09-08 16:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-07 18:19 [PATCH] config.mak.dev: add -Wformat-security Jeff King
2018-09-08 13:38 ` Duy Nguyen
2018-09-08 16:23 ` [PATCH v2] " Jeff King
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).