Jaysonyan added inline comments.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9230
+def warn_printf_n_specifier : Warning<
+ "usage of '%%n' can lead to unsafe writing to memory">,
InGroup<FormatNSpecifier>;
def warn_printf_data_arg_not_used : Warning<
----------------
aaron.ballman wrote:
> Quuxplusone wrote:
> > FWIW, I don't understand why this is "unsafe" either. The problem with `%n`
> > is not that it might be used //intentionally//; the problem is that it
> > opens up an attack vector for //unintentional// (malicious) use. The
> > programmer writes `printf(buf, args...)` where `buf` is under the
> > attacker's control (for example a debug-log format string supplied in a
> > config file), and then the //attacker// configures something like `"%n"`
> > instead of `"%s%d"` (so the debug-logging routine ends up poking data
> > instead of peeking it). This vulnerable `printf(buf, ...)` is exactly what
> > `-Wformat-security` warns about.
> > I am not aware of any vulnerability from //intentional// use of `%n`. At
> > best, one could argue that there's a moral hazard: we might like to remove
> > `%n`-support from our libc's printf, but we can't do that as long as
> > there's any code out there in the wild that relies on intentional use of
> > `%n`. Therefore, this is essentially a "deprecation warning" — but for a
> > feature that AFAIK has never been deprecated, neither by C nor C++! (Am I
> > wrong? Has someone officially deprecated `%n`?)
> FWIW, that's effectively how I view this as well -- it's kinda like `-Wvla`
> -- a diagnostic to say "congrats, you're using the feature!". However, unlike
> `-Wvla`, no one accidentally uses `%n` when they were expecting something
> else.
>
> `%n` isn't deprecated in either C or C++.
That's a really good point, I think you're right that this wouldn't be
effective in catching string format vulnerabilities, because as you said, the
attacker would need to control some portion of the format string which would
already be caught by `-Wformat-security`.
Although in fuchsia I think we still have a need for warning against even
explicit usages of `%n` since it could allow an external user to pass a
pointers into `printf("%n", ptr)` and write to addresses that we don't want to
be written to.
Thank you for the suggestion of clang-tidy. I think moving this warning to the
`bugprone` module sounds like a good way to surface a warning while not
impacting intended usages of `%n`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110436/new/
https://reviews.llvm.org/D110436
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
