[PATCH] D110436: Add %n format specifier warning

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 5 12:12:05 PDT 2021


aaron.ballman 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<
----------------
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++.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110436/new/

https://reviews.llvm.org/D110436



More information about the cfe-commits mailing list