<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div><br></div><div><div>On Mar 21, 2014, at 14:14 , Zach Davis <<a href="mailto:zdavkeos@gmail.com">zdavkeos@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">Background: Bug 18412 suggests that the compiler should issue a<br>security warning when a scanf %s format specifier does not<br>include a field width. This is the third patche working toward<br>this (r202114, 204300).<br><br>This patch adds the actual warning. The warning is part of the<br>FormatSecurity warning group.<br><br>Example:<br><br> test.c:14:10: warning: no field width in scanf string format<br>specifier (potentially insecure)<br> scanf("%s", str);<br> ^~<br><br>Presently one of the tests in test/Sema/format-strings-scanf.c<br>fails due to the way the tests are executed (the file is<br>re-compiled with the -Wformat=0 option). I would appreciate any<br>advice on fixing this test case.<br></blockquote><br></div><div>I think the right way to fix this is to put the field width into the fix-it when it's known. (Are you already doing that? If so, it needs tests!)</div><div><br></div><div><div>+#pragma clang diagnostic push // Don't warn about security problems.</div><div>+#pragma clang diagnostic ignored "-Wformat-security"</div><div> scanf("%lf", vstr);</div><div>+#pragma clang diagnostic pop</div><div><br></div><div>This seems wrong to me. We shouldn't emit the second warning if we're already fixing the format string.</div><div><br></div><div>Stepping back, I'm worried about adding this warning to <i>all</i> of -Wformat-security: in some code bases there may be perfectly innocuous uses of sscanf. Yes, you <i>could</i> add a sensible buffer width in pretty much all cases, but for existing codebases we probably don't want to deal with it.</div><div><br></div><div>How about emitting a warning directly in -Wformat-security if the buffer size is known, and in some new -Wformat-security-unknown-width (which is under -Wformat-security) if it isn't? That way people can turn off the warnings they can't immediately fix, but still see the obvious ones.</div><div><br></div><div>Jordan</div></div><br></body></html>