[PATCH] D110436: Add %n format specifier warning

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone 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<
----------------
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`?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110436