On Mar 21, 2014, at 14:14 , Zach Davis <[email protected]> wrote:

> Background: Bug 18412 suggests that the compiler should issue a
> security warning when a scanf %s format specifier does not
> include a field width.  This is the third patche working toward
> this (r202114, 204300).
> 
> This patch adds the actual warning. The warning is part of the
> FormatSecurity warning group.
> 
> Example:
> 
>    test.c:14:10: warning: no field width in scanf string format
> specifier (potentially insecure)
>      scanf("%s", str);
>             ^~
> 
> Presently one of the tests in test/Sema/format-strings-scanf.c
> fails due to the way the tests are executed (the file is
> re-compiled with the -Wformat=0 option). I would appreciate any
> advice on fixing this test case.

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!)

+#pragma clang diagnostic push // Don't warn about security problems.
+#pragma clang diagnostic ignored "-Wformat-security"
   scanf("%lf", vstr);
+#pragma clang diagnostic pop

This seems wrong to me. We shouldn't emit the second warning if we're already 
fixing the format string.

Stepping back, I'm worried about adding this warning to all of 
-Wformat-security: in some code bases there may be perfectly innocuous uses of 
sscanf. Yes, you could add a sensible buffer width in pretty much all cases, 
but for existing codebases we probably don't want to deal with it.

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.

Jordan

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to