clang is very noisy by default on projects using gettext

[llvmbot]

Bugzilla Link	3814
Version	unspecified
OS	Linux
Reporter	LLVM Bugzilla Contributor
CC	@lattner,@tkremenek,@pwo,@rjmccall

Extended Description

clang is very noisy by default on projects using gettext, because the format string literal warning is on by default, and it warns about code like

printf(_("some string"), ...);

where _ is a macro for some gettext function.

How crude would it be to recognize this pattern explicitly? I.e. printf of a direct call of a function taking a single literal string and returning a const char*.

[tkremenek]

clang is very noisy by default on projects using gettext, because the format
string literal warning is on by default, and it warns about code like

printf(_("some string"), ...);

where _ is a macro for some gettext function.

How crude would it be to recognize this pattern explicitly? I.e. printf of a
direct call of a function taking a single literal string and returning a const
char*.

Hmm. I personally have mixed feelings about hard-coding that pattern. Without knowing the implementation of the function, it could indeed return a different format string on every call. The "const char *" type is irrelevant.

[llvmbot]

Certainly, it could, the question is is it likely to be a security vulnerability.

The intent is to catch unintentional errors, not intentional ones; in this case we are passing a constant string literal to some function. Unless the programmer is trying to be malicious, it is likely that it will return something which only depends significantly on the input. In that case, it should be safe.

[lattner]

Doesn't gcc 4.2 -Wformat-security suffer from this same problem?

[tkremenek]

Certainly, it could, the question is is it likely to be a security
vulnerability.

The intent is to catch unintentional errors, not intentional ones; in this case
we are passing a constant string literal to some function. Unless the
programmer is trying to be malicious, it is likely that it will return
something which only depends significantly on the input. In that case, it
should be safe.

Couldn't they have just done:

printf("%s", foo(x))

instead of

prinf(foo(x))

The former is far more secure than the latter. That's the point of this warning.

[rjmccall]

(apologies if this was all hashed out a year ago out-of-band)

I agree that this is a false positive. The point of the diagnostic is to complain about things like printf(StringTheUserGaveMe) i.e. format arguments that aren't actually designed to be format strings. That's not happening here; the programmer is just picking a format string dynamically based on localization settings. This is an unsafe way to do localization, but it's not inherently insecure as long as the source of format strings is untampered with, and it doesn't deserve to be diagnosed.

That said, I don't know how to whitelist this case without introducing hacky gettext-specific workarounds. The nice fix is to somehow know that the function returns a format string (ideally this would be typed, but that's too much to ask).

Maybe we should only warn if there are no format arguments? That would still have some significant false positives; I'm not so worried about the false negatives.

[lattner]

John, what's the status of this old bug?

[llvmbot]

We still have the issue.

[llvmbot]

Not sure when the format string warnings were improved, but as of r158689 clang only warns on code like
printf(("some string"), ...);
if there are no additional arguments passed to printf (i.e. "printf(("foo"), 4);" won't have the warning but just "printf(_("foo"));" will), and the warning can be disabled entirely with -Wno-format-security.

[tkremenek]

Not sure when the format string warnings were improved, but as of r158689 clang
only warns on code like
printf(_("some string"), ...);

What is this syntax? Clang thinks that _(...) is a function call.

[llvmbot]

Not sure when the format string warnings were improved, but as of r158689 clang
only warns on code like
printf(_("some string"), ...);

What is this syntax? Clang thinks that _(...) is a function call.

I'm not sure about the actual gettext case, but I tested the warning with:

#include <stdio.h>
char* (const char*);
void test () {
printf(("some string"), 5);
printf(_("some string"));
}

And only the second printf triggered the -Wformat-security warning.

[lattner]

Clang is right. _ is the name of the function used by gettext to do localization remappings.

[llvmbot]

Yes, Clang is right, but I still think we should treat this as a false positive.

I don't think recognizing this pattern and suppressing the diagnostic for it would result in a large number of false negatives.

[lattner]

Completely agreed, just special casing "_" would be enough.