Sanitize printf functions

[kcc]

Tests, please.
We don't have a framework to run the same test under asan, tsan and msan,
so I guess we may have a full test in asan/lit_tests/TestCases/ and two tiny tests in
msan/lit_tests/ and tsan/lit_tests/

(FYI: I don't promise continued quick reviews until ~ Jan 10)

asan/asan_intercepted_functions.h

79 ↗

(On Diff #6284)

Why scanf?

asan/asan_interceptors.cc

678 ↗

(On Diff #6284)

yes, please

sanitizer_common/sanitizer_common_interceptors_scanf.inc

315 ↗

(On Diff #6284)

This interceptor will cause many new bugs to be discovered
and the deployment in many cases will take time.
Please add a flag "check_printf" to sanitizer_common/sanitizer_flags.h (true by default)
and if this flag is false, don't do any checking.

[kcc]

also, please CC llvm-commits

[ygribov]

Tests, please.

Yup, I forgot to do `svn add' when generating diff. Will do.

We don't have a framework to run the same test under asan, tsan and msan

Note that current version only intercepts printf for asan (interceptors are implemented in asan_interceptors.cc). I don't have much experience with msan so I'll need your advice on how to proceed there.

FYI I don't promise continued quick reviews until ~ Jan 10

Same here.

asan/asan_intercepted_functions.h

79 ↗

(On Diff #6284)

I felt cynical and simply grabbed SANITIZER_INTERCEPT_SCANF from sanitizer_common/sanitizer_platform_interceptors.h in hope that printf and scanf always go together. What about renaming SANITIZER_INTERCEPT_SCANF to SANITIZER_INTERCEPT_FORMAT?

asan/asan_interceptors.cc

678 ↗

(On Diff #6284)

I tried this initially but realized that Msan already intercepts printf stuff. What's the general policy about what goes into sanitizer_common (e.g. strcmp) and what gets re-implemented in each sanitizer (e.g. memset)?

sanitizer_common/sanitizer_common_interceptors_scanf.inc

315 ↗

(On Diff #6284)

Makes sense, will do.

[ygribov]

also, please CC llvm-commits

Done!

[kcc]

asan/asan_intercepted_functions.h

79 ↗

(On Diff #6284)

I would have a separate SANITIZER_INTERCEPT_PRINTF

asan/asan_interceptors.cc

678 ↗

(On Diff #6284)

msan interceptors are rudimentary and will have to be replaced with these *eventually*.
Here is a suggestion: move the code to sanitizer_common_interceptors.inc
but define SANITIZER_INTERCEPT_PRINTF to 1 only in asan.
tsan and msan will follow later.

[eugenis]

asan/asan_interceptors.cc

678 ↗

(On Diff #6284)

We can enable it in MSan from the start (killing the old msan interceptors), just make sure that the new interceptors emulate a WRITE to the target buffer (for sprintf and friends).

[ygribov]

asan/asan_interceptors.cc

678 ↗

(On Diff #6284)

Is current emulation fine or do you want to check each specifier separately?

[eugenis]

asan/asan_interceptors.cc

678 ↗

(On Diff #6284)

I mean, when this is moved to common interceptors, you'll need to do COMMON_INTERCEPTOR_WRITE_RANGE() for the sprintf target buffer, like the current msan interceptors do.
That is in addition to reads and writes that you do already.

[ygribov]

Updates based on prev review.

Missing stuff:

• MSan still using old interceptors
• no tests for TSan and MSan

Plus a bunch of questions inlined (see TODO markers).

[ygribov]

@Evgeny: let me know if this looks sane to you wrt MSan.

[kcc]

Let others look at this change too.
We won't commit it before the end of our holidays (~Jan 10) anyway.

Until we don't have tsan tests, I suggest to disable this for tsan.

msan/msan_interceptors.cc

1294 ↗

(On Diff #6286)

we can do it as a separate commit (I'd even prefer that)

sanitizer_common/sanitizer_common_interceptors.inc

33 ↗

(On Diff #6286)

can you avoid this?

sanitizer_common/sanitizer_common_interceptors_scanf.inc

281 ↗

(On Diff #6286)

good idea. feel free to implement or to leave the TODO.
Actually this code base uses FIXME (w/o name) more often.

[ygribov]

Let others look at this change too.
We won't commit it before the end of our holidays (~Jan 10) anyway.

Sure, there's no push from my side.

Until we don't have tsan tests, I suggest to disable this for tsan.

k

msan/msan_interceptors.cc

1294 ↗

(On Diff #6286)

Sure.

sanitizer_common/sanitizer_common_interceptors.inc

33 ↗

(On Diff #6286)

Well, I need size_t for snprintf.

sanitizer_common/sanitizer_common_interceptors_scanf.inc

281 ↗

(On Diff #6286)

Actually this code base uses FIXME (w/o name) more often.

Got it. Anyway, this was more a question for reviewers than real TODO. I'll add format check in next rev.

[kcc]

sanitizer_common/sanitizer_common_interceptors.inc

33 ↗

(On Diff #6286)

please use SIZE_T and don't include any additional system headers

[ygribov]

Until we don't have tsan tests, I suggest to disable this for tsan.

Will making a copy of printf-1.c for TSan be enough? Or you want a test with an actual race?

sanitizer_common/sanitizer_common_interceptors.inc

33 ↗

(On Diff #6286)

Agreed, my bad.

[glider]

sanitizer_common/sanitizer_common_interceptors_scanf.inc

10 ↗

(On Diff #6286)

Please update this comment.
Also, how about renaming the file to something like "sanitizer_common_interceptors_printf_scanf.inc" or "sanitizer_common_interceptors_format.inc"?

281 ↗

(On Diff #6286)

We sure need to check |format|. Since you've already touched scanf_common in this CL, why not just add

COMMON_INTERCEPTOR_READ_RANGE(ctx, format, internal_strlen(format) + 1);

?

362 ↗

(On Diff #6286)

Two spaces before the comment, please.

sanitizer_common/sanitizer_platform_interceptors.h

76 ↗

(On Diff #6286)

I wonder why the lines above have this strange indentation, but let us fix that in a separate commit.

[ygribov]

sanitizer_common/sanitizer_common_interceptors_scanf.inc

10 ↗

(On Diff #6286)

Please update this comment.

Will do.

Also, how about renaming the file to something like "sanitizer_common_interceptors_printf_scanf.inc"

But won't this make diff unreadable? Perhaps this should be a separate patch?

281 ↗

(On Diff #6286)

Yup, already added this in new version of the patch. I didn't resubmit it because I wanted to gather reviews from all team members.

The bigger question is whether we want to check size of input buffer in sscanf and friends.

BTW what's the general rule: should I resubmit patch as often as possible or wait till everyone comments?

362 ↗

(On Diff #6286)

Do you think we need a warning here? If yes, I'll add it, if not - I'll remove the comment alltogether.

sanitizer_common/sanitizer_platform_interceptors.h

76 ↗

(On Diff #6286)

Looks like something lint should be able to handle.

[glider]

sanitizer_common/sanitizer_common_interceptors_scanf.inc

10 ↗

(On Diff #6286)

But won't this make diff unreadable? Perhaps this should be a separate patch?

SGTM

281 ↗

(On Diff #6286)

BTW what's the general rule: should I resubmit patch as often as possible or wait till everyone comments?

I think it's better to resubmit once you've addressed a round of comments.
Most of the team is on holidays now, so you'll never know whether everyone is going to comment.

281 ↗

(On Diff #6286)

The bigger question is whether we want to check size of input buffer in sscanf and friends.

Yes, that's nice to have, too.

362 ↗

(On Diff #6286)

We can't get an invalid size from format_get_value_size(), so it's ok to have a CHECK here.

sanitizer_common/sanitizer_platform_interceptors.h

76 ↗

(On Diff #6286)

I've removed these. There's no clear policy about these spaces anyway.

[ygribov]

sanitizer_common/sanitizer_common_interceptors_scanf.inc

281 ↗

(On Diff #6286)

Any idea how to achieve this? Scanfs won't tell us number of bytes read and I can't make use of %n in interceptor...

362 ↗

(On Diff #6286)

Will do.

[ygribov]

Updated based on comments by glider and kcc.

[ygribov]

Will be in standby till Jan 9.

[glider]

Looks mostly good.

asan/lit_tests/TestCases/printf-4.c

6 ↗

(On Diff #6321)

Please sort the includes alphabetically (here and in other tests)

sanitizer_common/sanitizer_common_interceptors_scanf.inc

11 ↗

(On Diff #6321)

How about this line? Is there a printf specification we're following?

323 ↗

(On Diff #6321)

Please swap this line with the above declaration and remove the empty line below.

337 ↗

(On Diff #6321)

How does this comment relate to the above one?

350 ↗

(On Diff #6321)

Good point, probably worth doing.

[ygribov]

asan/lit_tests/TestCases/printf-4.c

6 ↗

(On Diff #6321)

Will do.

sanitizer_common/sanitizer_common_interceptors_scanf.inc

11 ↗

(On Diff #6321)

Oh, this is a good one. I've blindly reused scanf parser but it seems that printf format has richer syntax: it has flags and precision + asterisk semantics is different. I'll fix this in next rev.

323 ↗

(On Diff #6321)

+1

337 ↗

(On Diff #6321)

Sorry, not quite clear. I believe we don't want to bother with positional args neither in scanf, nor in printf.

350 ↗

(On Diff #6321)

Ok, I can do this. Note that there are actually many more checks of this sort: intersection of format with target string, intersection of format with %n destination, intersection of format with scanf argument, etc., etc. I guess errors like this should already be handled by things like FORTIFY_SOURCE pretty well.

[eugenis]

sanitizer_common/sanitizer_common_interceptors.inc

698 ↗

(On Diff #6321)

I'd remove check_printf from this condition, otherwise we'd get an MSan regression when check_printf is disabled. Also, these writes don't depend on format string parsing, and are most certainly correct.

The same goes to other v*printf interceptors.

sanitizer_common/sanitizer_common_interceptors_scanf.inc

11 ↗

(On Diff #6321)

Yes, this is kind of scary.
AFAIK, glibc has a comprehensive set of tests for scanf format string parsing. Jakub Jelinek ran this code on them and found several tricky bugs in the parser. Could you look into doing the same for printf?

[ygribov]

sanitizer_common/sanitizer_common_interceptors.inc

698 ↗

(On Diff #6321)

Note that check_printf is true by default and MSan will still use it's own interceptors for now.

sanitizer_common/sanitizer_common_interceptors_scanf.inc

11 ↗

(On Diff #6321)

AFAIK, glibc has a comprehensive set of tests for scanf format string parsing.

Are we talking about these:

$ find glibc-master -name tst\* -o -name test\* | grep printf
glibc-master/stdio-common/tst-swprintf.c
glibc-master/stdio-common/test-vfprintf.c
glibc-master/stdio-common/tst-printf-round.c
glibc-master/stdio-common/tst-printf.c
glibc-master/stdio-common/tst-sprintf3.c
glibc-master/stdio-common/tst-printf.sh
glibc-master/stdio-common/tst-sprintf2.c
glibc-master/stdio-common/tst-sprintf.c
glibc-master/stdio-common/tst-obprintf.c
glibc-master/stdio-common/tst-wc-printf.c
glibc-master/stdio-common/tst-printfsz.c

Could you look into doing the same for printf?

I probably should...

[kcc]

asan/lit_tests/TestCases/printf-2.c

2 ↗

(On Diff #6321)

please make these tests run in 3 modes:

ASAN_OPTIONS=check_printf=0
ASAN_OPTIONS=check_printf=1
ASAN_OPTIONS=""

we need to test that the flag affects the behavior and that it's default value is what we expect it to be.

(use FileCheck --check-prefix=CHECK-ON and --check-prefix=CHECK-OFF)

sanitizer_common/sanitizer_flags.h

60 ↗

(On Diff #6321)

I actually changed my mind.
Please make if false by default for now.
After this patch is committed we'll test it a bit more before enabling by default.

[ygribov]

asan/lit_tests/TestCases/printf-2.c

2 ↗

(On Diff #6321)

Ok.

[ygribov]

asan/lit_tests/TestCases/printf-2.c

2 ↗

(On Diff #6321)

BTW how do you suggest the check_printf=0 case? In my understanding we'll have UB with absolutely unpredictable behavior...

[ygribov]

asan/lit_tests/TestCases/printf-2.c

2 ↗

(On Diff #6321)

That's not that bad.
If you allocate a heap block using asan's allocator,
the behavior of an uncheck-ed out-of-bounds read is quite predictable

I tend to agree but e.g. GCC team tends to reject tests that trigger UB. Also things like this tend to be rather unstable with respect to glibc/kernel versions.

I'll do this if you're adamant though.

[ygribov]

New revision based on comments from glider, eugenis and kcc.

Main changes:

• fixed sanitizer_scanf_interceptor_test.cc
• update printf path based on Open Group std
• fixed several bugs based on glibc printf tests (manifested in segvs and false alarms)
• check_printf is now false by default

[eugenis]

lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc

214

According to the manual, field width can be "*", too. The number-or-star parsing logic could be factored out in a separate function.

222

This does not look right. %m in printf is a complete conversion specifier, it should end parsing of the current directive. And it seems like "m" could go into convSpecifier, and printErrno could be removed.

266

At this point allowGnuMalloc is true. It does not make sense for output to be true. Please remove this and instead check this invariant at the beginning of the functions (!(allowGnuMalloc && output)).

312

Are these guys ([...]) even allowed in printf?

363

Does it really need to be a macro?

[ygribov]

lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc

214

According to the manual, field width can be "*", too

Yup, this is handled by dir->suppressed above)

The number-or-star parsing logic could be factored out in a separate function.

Agreed.

222

%m in printf is a complete conversion specifier, it should end parsing of the current directive

I agree that we are currently parsing a wider format language than really necessary. But same is true for %n in original scanf implementation and we didn't bother handling it.

363

I was under impression that pointers to va_list are not portable C but http://gcc.gnu.org/ml/gcc-help/1999-q3n/msg00283.html suggests that they're sane. I'll give it a try tomorrow.

[ygribov]

lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc

266

Agreed.

312

No, they aren't. But why do you ask the question here?

[ygribov]

lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc

222

And it seems like "m" could go into convSpecifier, and printErrno could be removed.

Agreed to this one, will fix.

363

Ok, even though standard kind of allows this, GCC has a long and famous history of pointers to va_list not working (see e.g. http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14557). Looks like passing va_list by reference is unportable.

[eugenis]

lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc

214

Oh, right. This is a bit confusing, could you add a separate field to FormatDirective instead of reusing "suppressed" here?

222

What's wrong with %n? We parse it as convSpecifier which can not have anything going after it.
It seems like simply changing the code here to

if (*p == 'm' && !output) {

dir->allocate=true;
++p;

}

would do the trick.

312

Then it would make sense to handle this case earlier, in format_parse_next. Probably not a big deal.

[ygribov]

lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc

214

Already done. I've also decided to split format_parse_next to {printf,scanf}_parse_next because they are different enough.

222

Ok, should be fixed by splitting format_parse_next mentioned above.

[ygribov]

Major changes:

• split format_parse_next to 2 functions
• same for scanf_get_value_size
• safer check of buffer size in sprintf

Issues found by eugenis should be fixed now.

[eugenis]

lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc

204

It never returns null pointer.

212

maybe_parse_signed ?

420

Code duplication between scanf and printf parsers. Please factor this out.

[ygribov]

lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc

204

True, just trying to be safe here.

212

My bad.

420

Will do.

[ygribov]

Hey guys, I've fallen ill so won't be able to respond quickly.

Meanwhile, are there any other problems that you want fixed?

[eugenis]

I'll do another review pass tomorrow morning.
Get well!

[eugenis]

lib/sanitizer_common/sanitizer_common_interceptors.inc

697

Unless I'm missing something, you could use REAL(strlen)(str) here instead of calling *printf twice.

lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc

161

Should it be lengthModifier[0]?
Looks like a bug in the original code.

[ygribov]

lib/sanitizer_common/sanitizer_common_interceptors.inc

697

Hm but str isn't necessarily a string - it's just plain array of chars. I don't think we can call strlen on it.

lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc

161

1. Bug or some weird GNU scanf (mis)feature. Who is the author of this piece?
2. How do you guys normally treat situations when both change code and move it to a different place for clarity? Do you make two separate commits to make reviewers life easier?

[eugenis]

lib/sanitizer_common/sanitizer_common_interceptors.inc

697

I think *printf always end the output with a null character, and never print null in the middle, but I don't see a clear indication of that in the spec.

lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc

161

Me :)
I think it's a bug, it's meant to check that %p has no length modifier.
Two commits are good, but people often don't bother.
In this case it's ok to make the change in the same commit, or whatever is more convenient for you.

[ygribov]

lib/sanitizer_common/sanitizer_common_interceptors.inc

697

Sorry, I must be missing something... My intent with REAL(vname2) (which is actually REAL(vsnprintf)) is to estimate number of chars that will be written. 0 tells vsnprintf that it should not _write_ anything, only estimate number of chars. We could simply pass NULL instead of str and it wouldn't change anything. So why do you bother about str?

lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc

161

I'll update the code and see if tests seize to work then.

[eugenis]

lib/sanitizer_common/sanitizer_common_interceptors.inc

697

I'm just worried that it makes *printf quite a bit heavier - we now parse the format string 3 times (2 in libc *printf and 1 in our code). Alternatively, we could do 1 + 1, and then figure out the length of the resulting string with strlen() after the REAL *printf.

[ygribov]

lib/sanitizer_common/sanitizer_common_interceptors.inc

697

Agreed but then we may get stack overflow before Asan is able to react. Looks like a trade-off between speed and security. Does libsanitizer have general preference for any of those?

IMHO sprintf is already so slow that we may not need to bother so much.

[ygribov]

And sprintf usage is discouraged by all coding standards and static analyzers anyway :)

[eugenis]

lib/sanitizer_common/sanitizer_common_interceptors.inc

697

Now I must be missing something. What would be the cause of stack overflow?

That would slow down sprintf (and snprintf, etc) by a factor of 1.5 (roughly speaking). That may be a big hit exactly because sprintf is so slow. I'm assuming that calculation the length of the output by passing 0 as size takes about the same time as actually formatting the output.

[ygribov]

lib/sanitizer_common/sanitizer_common_interceptors.inc

697

Now I must be missing something. What would be the cause of stack overflow?

A call to REAL(sprintf) with unchecked target buffer size.

That would slow down sprintf (and snprintf, etc) by a factor of 1.5 (roughly speaking).

Yup, as I said I agree that this adds a penalty. Let's see if we can now come to conclusion on buffer overflow part and see which one is more important.

[eugenis]

lib/sanitizer_common/sanitizer_common_interceptors.inc

697

I see. But that's the same call that the application does, i.e. the overflow would have happened without sanitizer, too. And we will most likely catch it, but after sprintf corrupts some memory.

I think its common to use the strlen way. Interceptors like strcat, strcpy call the real function without checking that the string is actually null-terminated, and examine the result afterwards.

[ygribov]

lib/sanitizer_common/sanitizer_common_interceptors.inc

697

we will most likely catch it, but after sprintf corrupts some memory.

After some internal fighting I have to agree with that. The claim would be wrong on platforms with ascending stacks but I'm unaware of any living instance (last I heard Aarch64 failed at that).

I think its common to use the strlen way.
Interceptors like strcat, strcpy call the real function
without checking

I see, so we favor perf indeed. I'll remove the first sprintf call then.

BTW do we mention somewhere on our wiki that our interceptors are not 100% secure?

[ygribov]

New diff based on Evgeny's comments.

[eugenis]

Could you rebuild the new patch from projects/compiler-rt directory
instead of projects/compiler-rt/lib? Right now the incremental diff
between two last revisions is completely broken.

[ygribov]

Rebased diff to compiler-rt root.

[ygribov]

Frankly I'd expect Phabricator to catch silly errors like this...

[eugenis]

LGTM w/ nit
But please wait for samsonov's review.

lib/sanitizer_common/sanitizer_common_interceptors.inc

691

vname2 can be removed

697

I don't think we claim anywhere that ASan is 100% reliable, or that it catches all the bugs ;)

[samsonov]

lib/asan/lit_tests/TestCases/printf-1.c

5

Just use "%t 2>&1" to test it with default ASAN_OPTIONS.

7

I'd suggest to merge all these test cases into one, as we're really testing one thing here. That would save some lines of code and would make modifying and extending the test case easier, and the tests will run faster (you will compile/link a single binary instead of 5 binaries). You can control the printf(...) call you're testing with an argv. See TestCases/stack-buffer-overflow-with-position.cc

lib/asan/lit_tests/TestCases/printf-2.c

21

Could you match a file/line in the stack trace in error report? E.g. see TestCases/null_deref.cc

lib/sanitizer_common/sanitizer_common_interceptors.inc

573

Just "#include", we don't seem to use "# " indentation in another places in this file.

lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc

25

Quick check: is it OK you will consume the first "-" in "--" string?

30

Please clarify in function name or comment why "0" or "-0" are unexpected.

61

remove empty line

218

Is it ok the code originally parsed only unsigned integers as the field width, but now it will treat "-9" as a field width "9"?

348

I don't like this diagnostics: we don't even print what this unknown specifier is. We should either do this, or silently exit.

508

same here.

lib/sanitizer_common/tests/sanitizer_scanf_interceptor_test.cc

61

This one is also const.

[ygribov]

lib/asan/lit_tests/TestCases/printf-1.c

5

Ok.

7

I don't quite agree. Different tests trigger different types of memory errors so CHECKs will be different. Also printed messages in some tests differ.

lib/asan/lit_tests/TestCases/printf-2.c

21

Well, ok although I'd consider this an overkill.

lib/sanitizer_common/sanitizer_common_interceptors.inc

573

+1

691

+1

lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc

25

%-- is invalid syntax anyway so I think it's ok.

30

This is Evegeny's code so I have no idea.

61

Ok.

218

Well, the spec does not mention that width in scanf must be positive but GNU scanf rejects negatives. I'll replace with parse_number.

348

This warning is mainly to inform us that we failed to parse some format spec and I think it's rather important for debugging purposes. What if I replace it with VReport(1, ...) ?

lib/sanitizer_common/tests/sanitizer_scanf_interceptor_test.cc

61

+1

[eugenis]

lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc

30

Could it be different for printf vs scanf?

348

The warning is good, but to be useful it should print a bit more info. Otherwise in a big program it is not at all clear what to do about it.

[samsonov]

lib/asan/lit_tests/TestCases/printf-1.c

7

Well, ok, leaving this to you. If you think that merging the cases would complicate things too much, leave it as is.

lib/asan/lit_tests/TestCases/printf-2.c

21

We generally don't want many internal frames from ASan runtime to appear in the error report. So it's nice to verify that error report looks somewhat like this:
#0 __interceptor_printf ....
#1 main printf-2.c:16

lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc

25

Alright, I agree that %-- is just invalid scanf/printf syntax and we may not necessarily parse it correctly, but now we move this to function "maybe_parse_signed", and its name doesn't tell much about this hidden assumptions ("-" consumed from "--", "0" results in NULL returned), so one could use it in a wrong way somewhere else. IMO It's better to specify this assumptions explicitly here.

30

See above

[ygribov]

lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc

30

Closer study of scanf/printf spec reveals that width/precision are unsigned both for printf and scanf. But printf may have '-' or '+' flag in front of width.

Anyway I don't know why you check for <= 0 here.

348

The best solution would be to remember pointers to begin and end of specifier in format string. We can then use them to print specifier here.

[ygribov]

lib/asan/lit_tests/TestCases/printf-2.c

21

Got it.

lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc

25

I agree that whole parse_number/parse_signed thing is a bit messy. Let me see if I can prettify it with next commit.

[ygribov]

lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc

30

Ok, I got it: scanf is only allowed to have a non-zero width so this check needs to be moved to scanf_parse_next. All this syntax peculiarities are going to drive me crazy.

[ygribov]

Updates based on feedback from Evgeny&Alex.

What's missing:

• I haven't added the @LINE directives; currently ASan reports #0 0x431593 in printf_common #1 0x431a94 in __interceptor_vprintf #2 0x42b2fd in printf #3 0x47a897 in main

which probably is not what we want but I don't think that we should fix it in this patch.

Regarding glibc tests: I've ran them and verified that ASan does not cause segfaults or test failures (it does print couple of warnings on invalid format strings). Is this what you guys wanted?

[ygribov]

Alex, Evgeny,

Do you guys want anything else to go into this revision?

-Y

[samsonov]

I will look at it now. Sorry for delay.

[glider]

A couple of nits.

lib/sanitizer_common/tests/sanitizer_scanf_interceptor_test.cc

37–62

Can you please add a comment above describing why NOLINT is necessary here?

43

At least two spaces between the code and the comment, please (you'll need to reformat the rest of declarations)

[samsonov]

Only a few minor nits (I've also committed part of this patch already). I think it's fine for submission (once you're re-upload the diff addressing the comments).

lib/asan/lit_tests/TestCases/printf-5.c

5

Once again, what's the point of testing the default configuration? In this way you'll have to update the tests on a flag flip, is this really what we want? Can we remove it, or at least have a single test for default configuration?

lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc

48

q is always non-NULL

449

Please either make it a function (I see, va_list might be a problem), or name it as SKIP_SCALAR_ARG

lib/sanitizer_common/sanitizer_printf.cc

98 ↗

(On Diff #6503)

I've submitted this part in r199724 (it's not directly related to printf interceptors).

[kcc]

lib/asan/lit_tests/TestCases/printf-5.c

5

This was my suggestion, I think this *is* what we want.

[ygribov]

lib/asan/lit_tests/TestCases/printf-5.c

5

Yeah, I guess Kostya wants to explicitly test that printf checks are disabled by default for now.

lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc

48

+1

449

Will do.

lib/sanitizer_common/tests/sanitizer_scanf_interceptor_test.cc

37–62

I copy-pasted this from TEST(SanitizerCommonInterceptors, Scanf). Seems to work with NOLINT removed so I'll update the patch.

43

Likewise.

[ygribov]

Minor updates based on review.

[samsonov]

LGTM.

Submitted as r199729. Huge thanks for working on this!

[ygribov]

Np, waiting for real-world feedback from you.

lib/asan/lit_tests/TestCases/printf-1.c

+// RUN: %clang_asan -O2 %s -o %t
+// RUN: ASAN_OPTIONS=check_printf=1 %t 2>&1 | FileCheck %s
+// RUN: ASAN_OPTIONS=check_printf=0 %t 2>&1 | FileCheck %s
+// RUN: ASAN_OPTIONS="" %t 2>&1 | FileCheck %s
+
+#include <stdio.h>
+int main() {
+  volatile char c = '0';
+  volatile int x = 12;
+  volatile float f = 1.239;
+  volatile char s[] = "34";
+  printf("%c %d %.3f %s\n", c, x, f, s);
+  return 0;
+  // Check that printf works fine under Asan.
+  // CHECK: 0 12 1.239 34
+}

lib/asan/lit_tests/TestCases/printf-2.c

+// RUN: %clang_asan -O2 %s -o %t
+// RUN: ASAN_OPTIONS=check_printf=1 not %t 2>&1 | FileCheck --check-prefix=CHECK-ON %s
+// RUN: ASAN_OPTIONS=check_printf=0 %t 2>&1 | FileCheck --check-prefix=CHECK-OFF %s
+// RUN: ASAN_OPTIONS="" %t 2>&1 | FileCheck --check-prefix=CHECK-OFF %s
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+int main() {
+  volatile char c = '0';
+  volatile int x = 12;
+  volatile float f = 1.239;
+  volatile char s[] = "34";
+  char *p = strdup((const char *)s);
+  free(p);
+  printf("%c %d %.3f %s\n", c, x, f, p);
+  return 0;
+  // Check that %s is sanitized.
+  // CHECK-ON: heap-use-after-free
+  // CHECK-ON-NOT: 0 12 1.239 34
+  // CHECK-OFF: 0 12 1.239 
+}

lib/asan/lit_tests/TestCases/printf-3.c

+// RUN: %clang_asan -O2 %s -o %t
+// RUN: ASAN_OPTIONS=check_printf=1 not %t 2>&1 | FileCheck --check-prefix=CHECK-ON %s
+// RUN: ASAN_OPTIONS=check_printf=0 %t 2>&1 | FileCheck --check-prefix=CHECK-OFF %s
+// RUN: ASAN_OPTIONS="" %t 2>&1 | FileCheck --check-prefix=CHECK-OFF %s
+
+#include <stdio.h>
+int main() {
+  volatile char c = '0';
+  volatile int x = 12;
+  volatile float f = 1.239;
+  volatile char s[] = "34";
+  volatile int n[1];
+  printf("%c %d %.3f %s%n\n", c, x, f, s, &n[1]);
+  return 0;
+  // Check that %n is sanitized.
+  // CHECK-ON: stack-buffer-overflow
+  // CHECK-ON-NOT: 0 12 1.239 34
+  // CHECK-OFF: 0 12 1.239 34
+}

lib/asan/lit_tests/TestCases/printf-4.c

+// RUN: %clang_asan -O2 %s -o %t
+// RUN: ASAN_OPTIONS=check_printf=1 not %t 2>&1 | FileCheck --check-prefix=CHECK-ON %s
+// RUN: ASAN_OPTIONS=check_printf=0 %t 2>&1 | FileCheck --check-prefix=CHECK-OFF %s
+// RUN: ASAN_OPTIONS="" %t 2>&1 | FileCheck --check-prefix=CHECK-OFF %s
+
+#include <stdio.h>
+int main() {
+  volatile char c = '0';
+  volatile int x = 12;
+  volatile float f = 1.239;
+  volatile char s[] = "34";
+  volatile char buf[2];
+  sprintf((char *)buf, "%c %d %.3f %s\n", c, x, f, s);
+  puts((const char *)buf);
+  return 0;
+  // Check that size of output buffer is sanitized.
+  // CHECK-ON: stack-buffer-overflow
+  // CHECK-ON-NOT: 0 12 1.239 34
+  // CHECK-OFF: 0 12 1.239 34
+}

lib/asan/lit_tests/TestCases/printf-5.c

+// RUN: %clang_asan -O2 %s -o %t
+// RUN: ASAN_OPTIONS=check_printf=1 not %t 2>&1 | FileCheck --check-prefix=CHECK-ON %s
+// RUN: ASAN_OPTIONS=check_printf=0 %t 2>&1 | FileCheck --check-prefix=CHECK-OFF %s
+// RUN: ASAN_OPTIONS="" %t 2>&1 | FileCheck --check-prefix=CHECK-OFF %s
+
+#include <stdio.h>
+#include <string.h>
+int main() {
+  volatile char c = '0';
+  volatile int x = 12;
+  volatile float f = 1.239;
+  volatile char s[] = "34";
+  volatile char fmt[2];
+  memcpy((char *)fmt, "%c %d %f %s\n", sizeof(fmt));
+  printf(fmt, c, x, f, s);
+  return 0;
+  // Check that format string is sanitized.
+  // CHECK-ON: stack-buffer-overflow
+  // CHECK-ON-NOT: 0 12 1.239 34
+  // CHECK-OFF: 0
+}

lib/msan/msan_interceptors.cc

 #define COMMON_INTERCEPTOR_SET_THREAD_NAME(ctx, name) \
   do {                                                \
   } while (false)  // FIXME
 #define COMMON_INTERCEPTOR_SET_PTHREAD_NAME(ctx, thread, name) \
   do {                                                         \
   } while (false)  // FIXME
 #define COMMON_INTERCEPTOR_BLOCK_REAL(name) REAL(name)
 #define COMMON_INTERCEPTOR_ON_EXIT(ctx) OnExit()
+// FIXME: update Msan to use common printf interceptors
+#define SANITIZER_INTERCEPT_PRINTF 0
 #include "sanitizer_common/sanitizer_common_interceptors.inc"
 
 #define COMMON_SYSCALL_PRE_READ_RANGE(p, s) CHECK_UNPOISONED(p, s)
 #define COMMON_SYSCALL_PRE_WRITE_RANGE(p, s) \
   do {                                       \
   } while (false)
 #define COMMON_SYSCALL_POST_READ_RANGE(p, s) \
   do {                                       \

lib/sanitizer_common/sanitizer_common_interceptors.inc

INTERCEPTOR(char *, strptime, char *s, char *format, __sanitizer_tm *tm) {
   }
   return res;
 }
 #define INIT_STRPTIME COMMON_INTERCEPT_FUNCTION(strptime);
 #else
 #define INIT_STRPTIME
 #endif
 
-#if SANITIZER_INTERCEPT_SCANF
-
-#include "sanitizer_common_interceptors_scanf.inc"
+#if SANITIZER_INTERCEPT_SCANF || SANITIZER_INTERCEPT_PRINTF
+# include "sanitizer_common_interceptors_scanf.inc"
 
+#define FORMAT_INTERCEPTOR_IMPL(name, vname, ...)                              \
+  {                                                                            \
+    void *ctx;                                                                 \
+    va_list ap;                                                                \
+    va_start(ap, format);                                                      \
+    COMMON_INTERCEPTOR_ENTER(ctx, vname, __VA_ARGS__, ap);                     \
+    int res = vname(__VA_ARGS__, ap);                                          \
+    va_end(ap);                                                                \
+    return res;                                                                \
+  }
+
+#endif
+
+#if SANITIZER_INTERCEPT_SCANF
+
 #define VSCANF_INTERCEPTOR_IMPL(vname, allowGnuMalloc, ...)                    \
   {                                                                            \
     void *ctx;                                                                 \
     COMMON_INTERCEPTOR_ENTER(ctx, vname, __VA_ARGS__);                         \
     va_list aq;                                                                \
     va_copy(aq, ap);                                                           \
     int res = REAL(vname)(__VA_ARGS__);                                        \
     if (res > 0)                                                               \

 INTERCEPTOR(int, __isoc99_vsscanf, const char *str, const char *format,
             va_list ap)
 VSCANF_INTERCEPTOR_IMPL(__isoc99_vsscanf, false, str, format, ap)
 
 INTERCEPTOR(int, __isoc99_vfscanf, void *stream, const char *format, va_list ap)
 VSCANF_INTERCEPTOR_IMPL(__isoc99_vfscanf, false, stream, format, ap)
 #endif  // SANITIZER_INTERCEPT_ISOC99_SCANF
 
-#define SCANF_INTERCEPTOR_IMPL(name, vname, ...)                               \
-  {                                                                            \
-    void *ctx;                                                                 \
-    va_list ap;                                                                \
-    va_start(ap, format);                                                      \
-    COMMON_INTERCEPTOR_ENTER(ctx, vname, __VA_ARGS__, ap);                     \
-    int res = vname(__VA_ARGS__, ap);                                          \
-    va_end(ap);                                                                \
-    return res;                                                                \
-  }
-
 INTERCEPTOR(int, scanf, const char *format, ...)
-SCANF_INTERCEPTOR_IMPL(scanf, vscanf, format)
+FORMAT_INTERCEPTOR_IMPL(scanf, vscanf, format)
 
 INTERCEPTOR(int, fscanf, void *stream, const char *format, ...)
-SCANF_INTERCEPTOR_IMPL(fscanf, vfscanf, stream, format)
+FORMAT_INTERCEPTOR_IMPL(fscanf, vfscanf, stream, format)
 
 INTERCEPTOR(int, sscanf, const char *str, const char *format, ...)
-SCANF_INTERCEPTOR_IMPL(sscanf, vsscanf, str, format)
+FORMAT_INTERCEPTOR_IMPL(sscanf, vsscanf, str, format)
 
 #if SANITIZER_INTERCEPT_ISOC99_SCANF
 INTERCEPTOR(int, __isoc99_scanf, const char *format, ...)
-SCANF_INTERCEPTOR_IMPL(__isoc99_scanf, __isoc99_vscanf, format)
+FORMAT_INTERCEPTOR_IMPL(__isoc99_scanf, __isoc99_vscanf, format)
 
 INTERCEPTOR(int, __isoc99_fscanf, void *stream, const char *format, ...)
-SCANF_INTERCEPTOR_IMPL(__isoc99_fscanf, __isoc99_vfscanf, stream, format)
+FORMAT_INTERCEPTOR_IMPL(__isoc99_fscanf, __isoc99_vfscanf, stream, format)
 
 INTERCEPTOR(int, __isoc99_sscanf, const char *str, const char *format, ...)
-SCANF_INTERCEPTOR_IMPL(__isoc99_sscanf, __isoc99_vsscanf, str, format)
+FORMAT_INTERCEPTOR_IMPL(__isoc99_sscanf, __isoc99_vsscanf, str, format)
 #endif
 
 #endif
 
 #if SANITIZER_INTERCEPT_SCANF
 #define INIT_SCANF                    \
   COMMON_INTERCEPT_FUNCTION(scanf);   \
   COMMON_INTERCEPT_FUNCTION(sscanf);  \
#define INIT_ISOC99_SCANF                      \
   COMMON_INTERCEPT_FUNCTION(__isoc99_fscanf);  \
   COMMON_INTERCEPT_FUNCTION(__isoc99_vscanf);  \
   COMMON_INTERCEPT_FUNCTION(__isoc99_vsscanf); \
   COMMON_INTERCEPT_FUNCTION(__isoc99_vfscanf);
 #else
 #define INIT_ISOC99_SCANF
 #endif
 
+#if SANITIZER_INTERCEPT_PRINTF
+
+#define VPRINTF_INTERCEPTOR_ENTER(vname, ...)                                  \
+  void *ctx;                                                                   \
+  COMMON_INTERCEPTOR_ENTER(ctx, vname, __VA_ARGS__);                           \
+  va_list aq;                                                                  \
+  va_copy(aq, ap);
+
+#define VPRINTF_INTERCEPTOR_RETURN()                                           \
+  va_end(aq);
+
+#define VPRINTF_INTERCEPTOR_IMPL(vname, ...)                                   \
+  {                                                                            \
+    VPRINTF_INTERCEPTOR_ENTER(vname, __VA_ARGS__);                             \
+    if (common_flags()->check_printf)                                          \
+      printf_common(ctx, format, aq);                                          \
+    int res = REAL(vname)(__VA_ARGS__);                                        \
+    VPRINTF_INTERCEPTOR_RETURN();                                              \
+    return res;                                                                \
+  }
+
+#define VSPRINTF_INTERCEPTOR_IMPL(vname, vname2, str, ...)                     \
+  {                                                                            \
+    VPRINTF_INTERCEPTOR_ENTER(vname, str, __VA_ARGS__)                         \
+    if (common_flags()->check_printf) {                                        \
+      va_list ar;                                                              \
+      va_copy(ar, ap);                                                         \
+      int size = REAL(vname2)(str, 0, format, ar);                             \
+      if (size >= 0) {                                                         \
+        COMMON_INTERCEPTOR_WRITE_RANGE(ctx, str, size + 1);                    \
+      }                                                                        \
+      printf_common(ctx, format, aq);                                          \
+    }                                                                          \
+    int res = REAL(vname)(str, __VA_ARGS__);                                   \
+    VPRINTF_INTERCEPTOR_RETURN();                                              \
+    return res;                                                                \
+  }
+
+#define VSNPRINTF_INTERCEPTOR_IMPL(vname, str, size, ...)                      \
+  {                                                                            \
+    VPRINTF_INTERCEPTOR_ENTER(vname, str, size, __VA_ARGS__)                   \
+    if (common_flags()->check_printf) {                                        \
+      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, str, size);                          \
+      printf_common(ctx, format, aq);                                          \
+    }                                                                          \
+    int res = REAL(vname)(str, size, __VA_ARGS__);                             \
+    VPRINTF_INTERCEPTOR_RETURN();                                              \
+    return res;                                                                \
+  }
+
+#define VASPRINTF_INTERCEPTOR_IMPL(vname, strp, ...)                           \
+  {                                                                            \
+    VPRINTF_INTERCEPTOR_ENTER(vname, strp, __VA_ARGS__)                        \
+    if (common_flags()->check_printf) {                                        \
+      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, strp, sizeof(char *));               \
+      printf_common(ctx, format, aq);                                          \
+    }                                                                          \
+    int res = REAL(vname)(strp, __VA_ARGS__);                                  \
+    if (res >= 0 && common_flags()->check_printf) {                            \
+      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, *strp, res + 1);                     \
+    }                                                                          \
+    VPRINTF_INTERCEPTOR_RETURN();                                              \
+    return res;                                                                \
+  }
+
+INTERCEPTOR(int, vprintf, const char *format, va_list ap)
+VPRINTF_INTERCEPTOR_IMPL(vprintf, format, ap)
+
+INTERCEPTOR(int, vfprintf, void *stream, const char *format, va_list ap)
+VPRINTF_INTERCEPTOR_IMPL(vfprintf, stream, format, ap)
+
+INTERCEPTOR(int, vsnprintf, char *str, SIZE_T size, const char *format,
+            va_list ap)
+VSNPRINTF_INTERCEPTOR_IMPL(vsnprintf, str, size, format, ap)
+
+INTERCEPTOR(int, vsprintf, char *str, const char *format, va_list ap)
+VSPRINTF_INTERCEPTOR_IMPL(vsprintf, vsnprintf, str, format, ap)
+
+INTERCEPTOR(int, vasprintf, char **strp, const char *format, va_list ap)
+VASPRINTF_INTERCEPTOR_IMPL(vasprintf, strp, format, ap)
+
+#if SANITIZER_INTERCEPT_ISOC99_PRINTF
+INTERCEPTOR(int, __isoc99_vprintf, const char *format, va_list ap)
+VPRINTF_INTERCEPTOR_IMPL(__isoc99_vprintf, format, ap)
+
+INTERCEPTOR(int, __isoc99_vfprintf, void *stream, const char *format,
+            va_list ap)
+VPRINTF_INTERCEPTOR_IMPL(__isoc99_vfprintf, stream, format, ap)
+
+INTERCEPTOR(int, __isoc99_vsnprintf, char *str, SIZE_T size, const char *format,
+            va_list ap)
+VSNPRINTF_INTERCEPTOR_IMPL(__isoc99_vsnprintf, str, size, format, ap)
+
+INTERCEPTOR(int, __isoc99_vsprintf, char *str, const char *format,
+            va_list ap)
+VSPRINTF_INTERCEPTOR_IMPL(__isoc99_vsprintf, __isoc99_vsnprintf, str, format,
+                          ap)
+
+#endif  // SANITIZER_INTERCEPT_ISOC99_PRINTF
+
+INTERCEPTOR(int, printf, const char *format, ...)
+FORMAT_INTERCEPTOR_IMPL(printf, vprintf, format)
+
+INTERCEPTOR(int, fprintf, void *stream, const char *format, ...)
+FORMAT_INTERCEPTOR_IMPL(fprintf, vfprintf, stream, format)
+
+INTERCEPTOR(int, sprintf, char *str, const char *format, ...) // NOLINT
+FORMAT_INTERCEPTOR_IMPL(sprintf, vsprintf, str, format) // NOLINT
+
+INTERCEPTOR(int, snprintf, char *str, SIZE_T size, const char *format, ...)
+FORMAT_INTERCEPTOR_IMPL(snprintf, vsnprintf, str, size, format)
+
+INTERCEPTOR(int, asprintf, char **strp, const char *format, ...)
+FORMAT_INTERCEPTOR_IMPL(asprintf, vasprintf, strp, format)
+
+#if SANITIZER_INTERCEPT_ISOC99_PRINTF
+INTERCEPTOR(int, __isoc99_printf, const char *format, ...)
+FORMAT_INTERCEPTOR_IMPL(__isoc99_printf, __isoc99_vprintf, format)
+
+INTERCEPTOR(int, __isoc99_fprintf, void *stream, const char *format, ...)
+FORMAT_INTERCEPTOR_IMPL(__isoc99_fprintf, __isoc99_vfprintf, stream, format)
+
+INTERCEPTOR(int, __isoc99_sprintf, char *str, const char *format, ...)
+FORMAT_INTERCEPTOR_IMPL(__isoc99_sprintf, __isoc99_vsprintf, str, format)
+
+INTERCEPTOR(int, __isoc99_snprintf, char *str, SIZE_T size,
+            const char *format, ...)
+FORMAT_INTERCEPTOR_IMPL(__isoc99_snprintf, __isoc99_vsnprintf, str, size,
+                        format)
+
+#endif  // SANITIZER_INTERCEPT_ISOC99_PRINTF
+
+#endif  // SANITIZER_INTERCEPT_PRINTF
+
+#if SANITIZER_INTERCEPT_PRINTF
+#define INIT_PRINTF                     \
+  COMMON_INTERCEPT_FUNCTION(printf);    \
+  COMMON_INTERCEPT_FUNCTION(sprintf);   \
+  COMMON_INTERCEPT_FUNCTION(snprintf);  \
+  COMMON_INTERCEPT_FUNCTION(asprintf);  \
+  COMMON_INTERCEPT_FUNCTION(fprintf);   \
+  COMMON_INTERCEPT_FUNCTION(vprintf);   \
+  COMMON_INTERCEPT_FUNCTION(vsprintf);  \
+  COMMON_INTERCEPT_FUNCTION(vsnprintf); \
+  COMMON_INTERCEPT_FUNCTION(vasprintf); \
+  COMMON_INTERCEPT_FUNCTION(vfprintf);
+#else
+#define INIT_PRINTF
+#endif
+
+#if SANITIZER_INTERCEPT_ISOC99_PRINTF
+#define INIT_ISOC99_PRINTF                       \
+  COMMON_INTERCEPT_FUNCTION(__isoc99_printf);    \
+  COMMON_INTERCEPT_FUNCTION(__isoc99_sprintf);   \
+  COMMON_INTERCEPT_FUNCTION(__isoc99_snprintf);  \
+  COMMON_INTERCEPT_FUNCTION(__isoc99_fprintf);   \
+  COMMON_INTERCEPT_FUNCTION(__isoc99_vprintf);   \
+  COMMON_INTERCEPT_FUNCTION(__isoc99_vsprintf);  \
+  COMMON_INTERCEPT_FUNCTION(__isoc99_vsnprintf); \
+  COMMON_INTERCEPT_FUNCTION(__isoc99_vfprintf);
+#else
+#define INIT_ISOC99_PRINTF
+#endif
+
 #if SANITIZER_INTERCEPT_IOCTL
 #include "sanitizer_common_interceptors_ioctl.inc"
 INTERCEPTOR(int, ioctl, int d, unsigned request, void *arg) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, ioctl, d, request, arg);
 
   CHECK(ioctl_initialized);
 
#define SANITIZER_COMMON_INTERCEPTORS_INIT \
   INIT_WRITEV;                             \
   INIT_PWRITEV;                            \
   INIT_PWRITEV64;                          \
   INIT_PRCTL;                              \
   INIT_LOCALTIME_AND_FRIENDS;              \
   INIT_STRPTIME;                           \
   INIT_SCANF;                              \
   INIT_ISOC99_SCANF;                       \
+  INIT_PRINTF;                             \
+  INIT_ISOC99_PRINTF;                      \
   INIT_FREXP;                              \
   INIT_FREXPF_FREXPL;                      \
   INIT_GETPWNAM_AND_FRIENDS;               \
   INIT_GETPWNAM_R_AND_FRIENDS;             \
   INIT_CLOCK_GETTIME;                      \
   INIT_GETITIMER;                          \
   INIT_TIME;                               \
   INIT_GLOB;                               \

lib/sanitizer_common/sanitizer_common_interceptors_scanf.inc

 //===-- sanitizer_common_interceptors_scanf.inc -----------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
-// Scanf implementation for use in *Sanitizer interceptors.
+// Scanf/printf implementation for use in *Sanitizer interceptors.
 // Follows http://pubs.opengroup.org/onlinepubs/9699919799/functions/fscanf.html
+// and http://pubs.opengroup.org/onlinepubs/9699919799/functions/fprintf.html
 // with a few common GNU extensions.
 //
 //===----------------------------------------------------------------------===//
 #include <stdarg.h>
 
-struct ScanfDirective {
-  int argIdx; // argument index, or -1 of not specified ("%n$")
-  int fieldWidth;
-  bool suppressed; // suppress assignment ("*")
-  bool allocate;   // allocate space ("m")
-  char lengthModifier[2];
-  char convSpecifier;
-  bool maybeGnuMalloc;
-};
-
 static const char *parse_number(const char *p, int *out) {
   *out = internal_atoll(p);
   while (*p >= '0' && *p <= '9')
     ++p;
   return p;
 }
 
+static const char *maybe_parse_signed(const char *p, int *out) {
+  if (*p == '-')
+      ++p;
+  if (*p >= '0' && *p <= '9') {
+    p = parse_number(p, out);
+    if (*out <= 0)
+      return 0;
+  }
+  return p;
+}
+
+static const char *maybe_parse_signed_or_star(const char *p, int *out,
+                                              bool *star) {
+  if (*p == '*') {
+    *star = true;
+    ++p;
+  } else {
+    *star = false;
+    p = maybe_parse_signed(p, out);
+  }
+  return p;
+}
+
+static const char *maybe_parse_param_index(const char *p, int *out) {
+  // n$
+  if (*p >= '0' && *p <= '9') {
+    int number;
+    const char *q = parse_number(p, &number);
+    if (*q == '$') {
+      *out = number;
+      p = q + 1;
+    }
+  }
+
+  // Otherwise, do not change p. This will be re-parsed later as the field
+  // width.
+
+  return p;
+}
+
 static bool char_is_one_of(char c, const char *s) {
   return !!internal_strchr(s, c);
 }
 
+// Returns true if the character is an integer conversion specifier.
+static bool format_is_integer_conv(char c) {
+  return char_is_one_of(c, "diouxXn");
+}
+
+// Returns true if the character is an floating point conversion specifier.
+static bool format_is_float_conv(char c) {
+  return char_is_one_of(c, "aAeEfFgG");
+}
+
+// Returns string output character size for string-like conversions,
+// or 0 if the conversion is invalid.
+static int format_get_char_size(char convSpecifier,
+                                const char lengthModifier[2]) {
+  if (char_is_one_of(convSpecifier, "CS")) {
+    // wchar_t
+    return 0;
+  }
+
+  if (char_is_one_of(convSpecifier, "cs[")) {
+    if (lengthModifier[0] == 'l')
+      // wchar_t
+      return 0;
+    else if (lengthModifier[0] == 0)
+      return sizeof(char);
+    else
+      return 0;
+  }
+
+  return 0;
+}
+
+enum FormatStoreSize {
+  // Store size not known in advance; can be calculated as strlen() of the
+  // destination buffer.
+  FSS_STRLEN = -1,
+  // Invalid conversion specifier.
+  FSS_INVALID = 0
+};
+
+// Returns the memory size of a format directive (if >0), or a value of
+// FormatStoreSize.
+static int format_get_value_size(char convSpecifier,
+                                 const char lengthModifier[2], int fieldWidth,
+                                 bool promote_float) {
+  if (format_is_integer_conv(convSpecifier)) {
+    switch (lengthModifier[0]) {
+    case 'h':
+      return lengthModifier[1] == 'h' ? sizeof(char) : sizeof(short);
+    case 'l':
+      return lengthModifier[1] == 'l' ? sizeof(long long) : sizeof(long);
+    case 'L':
+      return sizeof(long long);
+    case 'j':
+      return sizeof(INTMAX_T);
+    case 'z':
+      return sizeof(SIZE_T);
+    case 't':
+      return sizeof(PTRDIFF_T);
+    case 0:
+      return sizeof(int);
+    default:
+      return FSS_INVALID;
+    }
+  }
+
+  if (format_is_float_conv(convSpecifier)) {
+    switch (lengthModifier[0]) {
+    case 'L':
+    case 'q':
+      return sizeof(long double);
+    case 'l':
+      return lengthModifier[1] == 'l' ? sizeof(long double)
+                                           : sizeof(double);
+    case 0:
+      // Printf promotes floats to doubles but scanf does not
+      return promote_float ? sizeof(double) : sizeof(float);
+    default:
+      return FSS_INVALID;
+    }
+  }
+
+  if (char_is_one_of(convSpecifier, "cC")) {
+    unsigned charSize = format_get_char_size(convSpecifier, lengthModifier);
+    if (charSize == 0)
+      return FSS_INVALID;
+    if (fieldWidth == 0)
+      return charSize;
+    return fieldWidth * charSize;
+  }
+
+  if (convSpecifier == 'p') {
+    if (lengthModifier[1] != 0)
+      return FSS_INVALID;
+    return sizeof(void *);
+  }
+
+  return FSS_INVALID;
+}
+
+struct ScanfDirective {
+  int argIdx; // argument index, or -1 if not specified ("%n$")
+  int fieldWidth;
+  bool suppressed; // suppress assignment ("*")
+  bool allocate;   // allocate space ("m")
+  char lengthModifier[2];
+  char convSpecifier;
+  bool maybeGnuMalloc;
+};
+
 // Parse scanf format string. If a valid directive in encountered, it is
 // returned in dir. This function returns the pointer to the first
 // unprocessed character, or 0 in case of error.
 // In case of the end-of-string, a pointer to the closing \0 is returned.
 static const char *scanf_parse_next(const char *p, bool allowGnuMalloc,
                                     ScanfDirective *dir) {
   internal_memset(dir, 0, sizeof(*dir));
   dir->argIdx = -1;
 
   while (*p) {
     if (*p != '%') {
       ++p;
       continue;
     }
     ++p;
     // %%
     if (*p == '%') {
       ++p;
       continue;
     }
     if (*p == '\0') {
       return 0;
     }
     // %n$
-    if (*p >= '0' && *p <= '9') {
-      int number;
-      const char *q = parse_number(p, &number);
-      if (*q == '$') {
-        dir->argIdx = number;
-        p = q + 1;
-      }
-      // Otherwise, do not change p. This will be re-parsed later as the field
-      // width.
-    }
+    p = maybe_parse_param_index(p, &dir->argIdx);
+    if (!p)
+      return 0;
     // *
     if (*p == '*') {
       dir->suppressed = true;
       ++p;
     }
     // Field width.
+    if (*p == '-')
+      ++p;
     if (*p >= '0' && *p <= '9') {
       p = parse_number(p, &dir->fieldWidth);
       if (dir->fieldWidth <= 0)
         return 0;
     }
     // m
     if (*p == 'm') {
       dir->allocate = true;
       ++p;
     }
     // Length modifier.
     if (char_is_one_of(*p, "jztLq")) {
       dir->lengthModifier[0] = *p;
       ++p;
     } else if (*p == 'h') {
       dir->lengthModifier[0] = 'h';
       ++p;
while (*p) {
     // This is unfortunately ambiguous between old GNU extension
     // of %as, %aS and %a[...] and newer POSIX %a followed by
     // letters s, S or [.
     if (allowGnuMalloc && dir->convSpecifier == 'a' &&
         !dir->lengthModifier[0]) {
       if (*p == 's' || *p == 'S') {
         dir->maybeGnuMalloc = true;
         ++p;
       } else if (*p == '[') {
         // Watch for %a[h-j%d], if % appears in the
         // [...] range, then we need to give up, we don't know
         // if scanf will parse it as POSIX %a [h-j %d ] or
         // GNU allocation of string with range dh-j plus %.
         const char *q = p + 1;
         if (*q == '^')
           ++q;
         if (*q == ']')
           ++q;
         while (*q && *q != ']' && *q != '%')
           ++q;
         if (*q == 0 || *q == '%')
           return 0;
         p = q + 1; // Consume the closing ']'.
         dir->maybeGnuMalloc = true;
       }
     }
     break;
   }
   return p;
 }
 
-// Returns true if the character is an integer conversion specifier.
-static bool scanf_is_integer_conv(char c) {
-  return char_is_one_of(c, "diouxXn");
-}
-
-// Returns true if the character is an floating point conversion specifier.
-static bool scanf_is_float_conv(char c) {
-  return char_is_one_of(c, "aAeEfFgG");
-}
-
-// Returns string output character size for string-like conversions,
-// or 0 if the conversion is invalid.
-static int scanf_get_char_size(ScanfDirective *dir) {
-  if (char_is_one_of(dir->convSpecifier, "CS")) {
-    // wchar_t
-    return 0;
-  }
-
-  if (char_is_one_of(dir->convSpecifier, "cs[")) {
-    if (dir->lengthModifier[0] == 'l')
-      // wchar_t
-      return 0;
-    else if (dir->lengthModifier[0] == 0)
-      return sizeof(char);
-    else
-      return 0;
-  }
-
-  return 0;
-}
-
-enum ScanfStoreSize {
-  // Store size not known in advance; can be calculated as strlen() of the
-  // destination buffer.
-  SSS_STRLEN = -1,
-  // Invalid conversion specifier.
-  SSS_INVALID = 0
-};
-
-// Returns the store size of a scanf directive (if >0), or a value of
-// ScanfStoreSize.
-static int scanf_get_store_size(ScanfDirective *dir) {
+static int scanf_get_value_size(ScanfDirective *dir) {
   if (dir->allocate) {
     if (!char_is_one_of(dir->convSpecifier, "cCsS["))
-      return SSS_INVALID;
+      return FSS_INVALID;
     return sizeof(char *);
   }
 
   if (dir->maybeGnuMalloc) {
     if (dir->convSpecifier != 'a' || dir->lengthModifier[0])
-      return SSS_INVALID;
+      return FSS_INVALID;
     // This is ambiguous, so check the smaller size of char * (if it is
     // a GNU extension of %as, %aS or %a[...]) and float (if it is
     // POSIX %a followed by s, S or [ letters).
     return sizeof(char *) < sizeof(float) ? sizeof(char *) : sizeof(float);
   }
 
-  if (scanf_is_integer_conv(dir->convSpecifier)) {
-    switch (dir->lengthModifier[0]) {
-    case 'h':
-      return dir->lengthModifier[1] == 'h' ? sizeof(char) : sizeof(short);
-    case 'l':
-      return dir->lengthModifier[1] == 'l' ? sizeof(long long) : sizeof(long);
-    case 'L':
-      return sizeof(long long);
-    case 'j':
-      return sizeof(INTMAX_T);
-    case 'z':
-      return sizeof(SIZE_T);
-    case 't':
-      return sizeof(PTRDIFF_T);
-    case 0:
-      return sizeof(int);
-    default:
-      return SSS_INVALID;
-    }
-  }
-
-  if (scanf_is_float_conv(dir->convSpecifier)) {
-    switch (dir->lengthModifier[0]) {
-    case 'L':
-    case 'q':
-      return sizeof(long double);
-    case 'l':
-      return dir->lengthModifier[1] == 'l' ? sizeof(long double)
-                                           : sizeof(double);
-    case 0:
-      return sizeof(float);
-    default:
-      return SSS_INVALID;
-    }
-  }
-
   if (char_is_one_of(dir->convSpecifier, "sS[")) {
-    unsigned charSize = scanf_get_char_size(dir);
+    unsigned charSize = format_get_char_size(dir->convSpecifier,
+                                             dir->lengthModifier);
     if (charSize == 0)
-      return SSS_INVALID;
+      return FSS_INVALID;
     if (dir->fieldWidth == 0)
-      return SSS_STRLEN;
+      return FSS_STRLEN;
     return (dir->fieldWidth + 1) * charSize;
   }
 
-  if (char_is_one_of(dir->convSpecifier, "cC")) {
-    unsigned charSize = scanf_get_char_size(dir);
-    if (charSize == 0)
-      return SSS_INVALID;
-    if (dir->fieldWidth == 0)
-      return charSize;
-    return dir->fieldWidth * charSize;
-  }
-
-  if (dir->convSpecifier == 'p') {
-    if (dir->lengthModifier[1] != 0)
-      return SSS_INVALID;
-    return sizeof(void *);
-  }
-
-  return SSS_INVALID;
+  return format_get_value_size(dir->convSpecifier, dir->lengthModifier,
+                               dir->fieldWidth, false);
 }
 
 // Common part of *scanf interceptors.
 // Process format string and va_list, and report all store ranges.
 // Stops when "consuming" n_inputs input items.
 static void scanf_common(void *ctx, int n_inputs, bool allowGnuMalloc,
                          const char *format, va_list aq) {
   CHECK_GT(n_inputs, 0);
   const char *p = format;
 
+  COMMON_INTERCEPTOR_READ_RANGE(ctx, format, internal_strlen(format) + 1);
+
   while (*p) {
     ScanfDirective dir;
     p = scanf_parse_next(p, allowGnuMalloc, &dir);
     if (!p)
       break;
     if (dir.convSpecifier == 0) {
       // This can only happen at the end of the format string.
       CHECK_EQ(*p, 0);
       break;
     }
     // Here the directive is valid. Do what it says.
     if (dir.argIdx != -1) {
       // Unsupported.
       break;
     }
     if (dir.suppressed)
       continue;
-    int size = scanf_get_store_size(&dir);
-    if (size == SSS_INVALID)
+    int size = scanf_get_value_size(&dir);
+    if (size == FSS_INVALID) {
+      Report("WARNING: unexpected format specifier in scanf interceptor\n");
       break;
+    }
     void *argp = va_arg(aq, void *);
     if (dir.convSpecifier != 'n')
       --n_inputs;
     if (n_inputs < 0)
       break;
-    if (size == SSS_STRLEN) {
+    if (size == FSS_STRLEN) {
       size = internal_strlen((const char *)argp) + 1;
     }
     COMMON_INTERCEPTOR_WRITE_RANGE(ctx, argp, size);
   }
 }
+
+struct PrintfDirective {
+  int fieldWidth;
+  int fieldPrecision;
+  int argIdx; // width argument index, or -1 if not specified ("%*n$")
+  int precisionIdx; // precision argument index, or -1 if not specified (".*n$")
+  bool starredWidth;
+  bool starredPrecision;
+  char lengthModifier[2];
+  char convSpecifier;
+};
+
+// Parse printf format string. Same as scanf_parse_next.
+static const char *printf_parse_next(const char *p, PrintfDirective *dir) {
+  internal_memset(dir, 0, sizeof(*dir));
+  dir->argIdx = dir->precisionIdx = -1;
+
+  while (*p) {
+    if (*p != '%') {
+      ++p;
+      continue;
+    }
+    ++p;
+    // %%
+    if (*p == '%') {
+      ++p;
+      continue;
+    }
+    if (*p == '\0') {
+      return 0;
+    }
+    // %n$
+    p = maybe_parse_param_index(p, &dir->precisionIdx);
+    if (!p)
+      return 0;
+    // Flags
+    while (char_is_one_of(*p, "'-+ #0")) {
+      ++p;
+    }
+    // Field width
+    p = maybe_parse_signed_or_star(p, &dir->fieldWidth, &dir->starredWidth);
+    if (!p)
+      return 0;
+    // Precision.
+    if (*p == '.') {
+      ++p;
+      p = maybe_parse_signed_or_star(p, &dir->fieldPrecision,
+                                     &dir->starredPrecision);
+      if (!p)
+        return 0;
+      // m$
+      if (dir->starredPrecision) {
+        p = maybe_parse_param_index(p, &dir->precisionIdx);
+        if (!p)
+          return 0;
+      }
+    }
+    // Length modifier.
+    if (char_is_one_of(*p, "jztLq")) {
+      dir->lengthModifier[0] = *p;
+      ++p;
+    } else if (*p == 'h') {
+      dir->lengthModifier[0] = 'h';
+      ++p;
+      if (*p == 'h') {
+        dir->lengthModifier[1] = 'h';
+        ++p;
+      }
+    } else if (*p == 'l') {
+      dir->lengthModifier[0] = 'l';
+      ++p;
+      if (*p == 'l') {
+        dir->lengthModifier[1] = 'l';
+        ++p;
+      }
+    }
+    // Conversion specifier.
+    dir->convSpecifier = *p++;
+    break;
+  }
+  return p;
+}
+
+static int printf_get_value_size(PrintfDirective *dir) {
+  if (dir->convSpecifier == 'm') {
+    return sizeof(char *);
+  }
+
+  if (char_is_one_of(dir->convSpecifier, "sS")) {
+    unsigned charSize = format_get_char_size(dir->convSpecifier,
+                                             dir->lengthModifier);
+    if (charSize == 0)
+      return FSS_INVALID;
+    return FSS_STRLEN;
+  }
+
+  return format_get_value_size(dir->convSpecifier, dir->lengthModifier,
+                               dir->fieldWidth, true);
+}
+
+#define skip_scalar_arg(aq, convSpecifier, size)                   \
+  do {                                                             \
+    if (format_is_float_conv(convSpecifier)) {                     \
+      switch (size) {                                              \
+      case 8:                                                      \
+        va_arg(*aq, double);                                       \
+        break;                                                     \
+      case 16:                                                     \
+        va_arg(*aq, long double);                                  \
+        break;                                                     \
+      default:                                                     \
+        Report("WARNING: unexpected floating-point arg size"       \
+               " in printf interceptor: %d\n", size);              \
+        return;                                                    \
+      }                                                            \
+    } else {                                                       \
+      switch (size) {                                              \
+      case 1:                                                      \
+      case 2:                                                      \
+      case 4:                                                      \
+        va_arg(*aq, u32);                                          \
+        break;                                                     \
+      case 8:                                                      \
+        va_arg(*aq, u64);                                          \
+        break;                                                     \
+      default:                                                     \
+        Report("WARNING: unexpected arg size"                      \
+               " in printf interceptor: %d\n", size);              \
+        return;                                                    \
+      }                                                            \
+    }                                                              \
+  } while (0)
+
+// Common part of *printf interceptors.
+// Process format string and va_list, and report all load ranges.
+static void printf_common(void *ctx, const char *format, va_list aq) {
+  COMMON_INTERCEPTOR_READ_RANGE(ctx, format, internal_strlen(format) + 1);
+
+  const char *p = format;
+
+  while (*p) {
+    PrintfDirective dir;
+    p = printf_parse_next(p, &dir);
+    if (!p)
+      break;
+    if (dir.convSpecifier == 0) {
+      // This can only happen at the end of the format string.
+      CHECK_EQ(*p, 0);
+      break;
+    }
+    // Here the directive is valid. Do what it says.
+    if (dir.argIdx != -1 || dir.precisionIdx != -1) {
+      // Unsupported.
+      break;
+    }
+    if (dir.starredWidth) {
+      // Dynamic width
+      skip_scalar_arg(&aq, 'd', sizeof(int));
+    }
+    if (dir.starredPrecision) {
+      // Dynamic precision
+      skip_scalar_arg(&aq, 'd', sizeof(int));
+    }
+    int size = printf_get_value_size(&dir);
+    if (size == FSS_INVALID) {
+      Report("WARNING: unexpected format specifier in printf interceptor\n");
+      break;
+    }
+    if (dir.convSpecifier == 'n') {
+      void *argp = va_arg(aq, void *);
+      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, argp, size);
+      continue;
+    } else if (size == FSS_STRLEN) {
+      if (void *argp = va_arg(aq, void *)) {
+        size = internal_strlen((const char *)argp) + 1;
+        COMMON_INTERCEPTOR_READ_RANGE(ctx, argp, size);
+      }
+    } else {
+      // Skip non-pointer args
+      skip_scalar_arg(&aq, dir.convSpecifier, size);
+    }
+  }
+}

lib/sanitizer_common/sanitizer_flags.h

struct CommonFlags {
   // Invoke leak checking in an atexit handler. Has no effect if
   // detect_leaks=false, or if __lsan_do_leak_check() is called before the
   // handler has a chance to run.
   bool leak_check_at_exit;
   // If false, the allocator will crash instead of returning 0 on out-of-memory.
   bool allocator_may_return_null;
   // If false, disable printing error summaries in addition to error reports.
   bool print_summary;
+  // Check printf arguments.
+  bool check_printf;
 };
 
 inline CommonFlags *common_flags() {
   static CommonFlags f;
   return &f;
 }
 
 void SetCommonFlagsDefaults(CommonFlags *f);
 void ParseCommonFlagsFromString(CommonFlags *f, const char *str);
 
 }  // namespace __sanitizer
 
 #endif  // SANITIZER_FLAGS_H

lib/sanitizer_common/sanitizer_flags.cc

void SetCommonFlagsDefaults(CommonFlags *f) {
   f->handle_ioctl = false;
   f->malloc_context_size = 1;
   f->log_path = "stderr";
   f->verbosity = 0;
   f->detect_leaks = false;
   f->leak_check_at_exit = true;
   f->allocator_may_return_null = false;
   f->print_summary = true;
+  f->check_printf = false;
 }
 
 void ParseCommonFlagsFromString(CommonFlags *f, const char *str) {
   ParseFlag(str, &f->symbolize, "symbolize");
   ParseFlag(str, &f->external_symbolizer_path, "external_symbolizer_path");
   ParseFlag(str, &f->strip_path_prefix, "strip_path_prefix");
   ParseFlag(str, &f->fast_unwind_on_fatal, "fast_unwind_on_fatal");
   ParseFlag(str, &f->fast_unwind_on_malloc, "fast_unwind_on_malloc");
   ParseFlag(str, &f->handle_ioctl, "handle_ioctl");
   ParseFlag(str, &f->malloc_context_size, "malloc_context_size");
   ParseFlag(str, &f->log_path, "log_path");
   ParseFlag(str, &f->verbosity, "verbosity");
   ParseFlag(str, &f->detect_leaks, "detect_leaks");
   ParseFlag(str, &f->leak_check_at_exit, "leak_check_at_exit");
   ParseFlag(str, &f->allocator_may_return_null, "allocator_may_return_null");
   ParseFlag(str, &f->print_summary, "print_summary");
+  ParseFlag(str, &f->check_printf, "check_printf");
 
   // Do a sanity check for certain flags.
   if (f->malloc_context_size < 1)
     f->malloc_context_size = 1;
   if (!f->symbolize)
     f->external_symbolizer_path = "";
 }
 

lib/sanitizer_common/sanitizer_platform_interceptors.h

 #define SANITIZER_INTERCEPT_PRCTL   SI_LINUX
 
 #define SANITIZER_INTERCEPT_LOCALTIME_AND_FRIENDS SI_NOT_WINDOWS
 #define SANITIZER_INTERCEPT_STRPTIME SI_NOT_WINDOWS
 
 #define SANITIZER_INTERCEPT_SCANF SI_NOT_WINDOWS
 #define SANITIZER_INTERCEPT_ISOC99_SCANF SI_LINUX
 
+#ifndef SANITIZER_INTERCEPT_PRINTF
+# define SANITIZER_INTERCEPT_PRINTF SI_NOT_WINDOWS
+# define SANITIZER_INTERCEPT_ISOC99_PRINTF SI_LINUX
+#endif
+
 #define SANITIZER_INTERCEPT_FREXP 1
 #define SANITIZER_INTERCEPT_FREXPF_FREXPL SI_NOT_WINDOWS
 
 #define SANITIZER_INTERCEPT_GETPWNAM_AND_FRIENDS SI_NOT_WINDOWS
 #define SANITIZER_INTERCEPT_GETPWNAM_R_AND_FRIENDS \
   SI_MAC || SI_LINUX_NOT_ANDROID
 #define SANITIZER_INTERCEPT_CLOCK_GETTIME SI_LINUX
 #define SANITIZER_INTERCEPT_GETITIMER SI_NOT_WINDOWS

lib/sanitizer_common/tests/sanitizer_scanf_interceptor_test.cc

 // Tests for *scanf interceptors implementation in sanitizer_common.
 //
 //===----------------------------------------------------------------------===//
 #include <vector>
 
 #include "interception/interception.h"
 #include "sanitizer_test_utils.h"
 #include "sanitizer_common/sanitizer_libc.h"
+#include "sanitizer_common/sanitizer_common.h"
 #include "gtest/gtest.h"
 
 using namespace __sanitizer;
 
+#define COMMON_INTERCEPTOR_READ_WRITE_RANGE(ctx, ptr, size)                    \
+  do {                                                                         \
+    ((std::vector<unsigned> *)ctx)->push_back(size);                           \
+    ptr = ptr;                                                                 \
+  } while (0)
+
+#define COMMON_INTERCEPTOR_READ_RANGE(ctx, ptr, size)                          \
+  COMMON_INTERCEPTOR_READ_WRITE_RANGE(ctx, ptr, size)
+
 #define COMMON_INTERCEPTOR_WRITE_RANGE(ctx, ptr, size)                         \
-  ((std::vector<unsigned> *)ctx)->push_back(size)
+  COMMON_INTERCEPTOR_READ_WRITE_RANGE(ctx, ptr, size)
 
 #include "sanitizer_common/sanitizer_common_interceptors_scanf.inc"
 
-static const char scanf_buf[] = "Test string.";
-static size_t scanf_buf_size = sizeof(scanf_buf);
+static const unsigned I = sizeof(int);          // NOLINT
+static const unsigned L = sizeof(long);         // NOLINT
+static const unsigned LL = sizeof(long long);   // NOLINT
+static const unsigned S = sizeof(short);        // NOLINT
+static const unsigned C = sizeof(char);         // NOLINT
+static const unsigned D = sizeof(double);       // NOLINT
+static const unsigned LD = sizeof(long double); // NOLINT
+static const unsigned F = sizeof(float);        // NOLINT
+static const unsigned P = sizeof(char *);       // NOLINT
+
+static void verifyFormatResults(const char *format, unsigned n,
+                                const std::vector<unsigned> &computed_sizes,
+                                va_list expected_sizes) {
+  // "+ 1" because of format string
+  ASSERT_EQ(n + 1,
+            computed_sizes.size()) << "Unexpected number of format arguments: '"
+                                   << format << "'";
+  for (unsigned i = 0; i < n; ++i)
+    EXPECT_EQ(va_arg(expected_sizes, unsigned), computed_sizes[i + 1])
+        << "Unexpect write size for argument " << i << ", format string '"
+        << format << "'";
+}
+
+static const char test_buf[] = "Test string.";
+static size_t test_buf_size = sizeof(test_buf);
+
 static const unsigned SCANF_ARGS_MAX = 16;
 
 static void testScanf3(void *ctx, int result, bool allowGnuMalloc,
                        const char *format, ...) {
   va_list ap;
   va_start(ap, format);
   scanf_common(ctx, result, allowGnuMalloc, format, ap);
   va_end(ap);
 }
 
 static void testScanf2(const char *format, int scanf_result,
                        bool allowGnuMalloc, unsigned n,
                        va_list expected_sizes) {
   std::vector<unsigned> scanf_sizes;
   // 16 args should be enough.
   testScanf3((void *)&scanf_sizes, scanf_result, allowGnuMalloc, format,
-             scanf_buf, scanf_buf, scanf_buf, scanf_buf, scanf_buf, scanf_buf,
-             scanf_buf, scanf_buf, scanf_buf, scanf_buf, scanf_buf, scanf_buf,
-             scanf_buf, scanf_buf, scanf_buf, scanf_buf);
-  ASSERT_EQ(n, scanf_sizes.size()) << "Unexpected number of format arguments: '"
-                                   << format << "'";
-  for (unsigned i = 0; i < n; ++i)
-    EXPECT_EQ(va_arg(expected_sizes, unsigned), scanf_sizes[i])
-        << "Unexpect write size for argument " << i << ", format string '"
-        << format << "'";
+             test_buf, test_buf, test_buf, test_buf, test_buf, test_buf,
+             test_buf, test_buf, test_buf, test_buf, test_buf, test_buf,
+             test_buf, test_buf, test_buf, test_buf);
+  verifyFormatResults(format, n, scanf_sizes, expected_sizes);
 }
 
 static void testScanf(const char *format, unsigned n, ...) {
   va_list ap;
   va_start(ap, n);
   testScanf2(format, SCANF_ARGS_MAX, /* allowGnuMalloc */ true, n, ap);
   va_end(ap);
 }

 static void testScanfNoGnuMalloc(const char *format, unsigned n, ...) {
   va_list ap;
   va_start(ap, n);
   testScanf2(format, SCANF_ARGS_MAX, /* allowGnuMalloc */ false, n, ap);
   va_end(ap);
 }
 
 TEST(SanitizerCommonInterceptors, Scanf) {
-  const unsigned I = sizeof(int);          // NOLINT
-  const unsigned L = sizeof(long);         // NOLINT
-  const unsigned LL = sizeof(long long);   // NOLINT
-  const unsigned S = sizeof(short);        // NOLINT
-  const unsigned C = sizeof(char);         // NOLINT
-  const unsigned D = sizeof(double);       // NOLINT
-  const unsigned LD = sizeof(long double); // NOLINT
-  const unsigned F = sizeof(float);        // NOLINT
-  const unsigned P = sizeof(char *);       // NOLINT
-
   testScanf("%d", 1, I);
   testScanf("%d%d%d", 3, I, I, I);
   testScanf("ab%u%dc", 2, I, I);
   testScanf("%ld", 1, L);
   testScanf("%llu", 1, LL);
   testScanf("a %hd%hhx", 2, S, C);
   testScanf("%c", 1, C);
 
TEST(SanitizerCommonInterceptors, Scanf) {
 
   testScanf("%10s", 1, 11);
   testScanf("%10c", 1, 10);
   testScanf("%%10s", 0);
   testScanf("%*10s", 0);
   testScanf("%*d", 0);
 
   testScanf("%4d%8f%c", 3, I, F, C);
-  testScanf("%s%d", 2, scanf_buf_size, I);
-  testScanf("%[abc]", 1, scanf_buf_size);
+  testScanf("%s%d", 2, test_buf_size, I);
+  testScanf("%[abc]", 1, test_buf_size);
   testScanf("%4[bcdef]", 1, 5);
-  testScanf("%[]]", 1, scanf_buf_size);
+  testScanf("%[]]", 1, test_buf_size);
   testScanf("%8[^]%d0-9-]%c", 2, 9, C);
 
   testScanf("%*[^:]%n:%d:%1[ ]%n", 4, I, I, 2, I);
 
   testScanf("%*d%u", 1, I);
 
   testScanf("%c%d", 2, C, I);
   testScanf("%A%lf", 2, F, D);
TEST(SanitizerCommonInterceptors, Scanf) {
   testScanfPartial("%d%d%d%d //1\n", 1, 1, I);
   testScanfPartial("%d%d%d%d //2\n", 2, 2, I, I);
   testScanfPartial("%d%d%d%d //3\n", 3, 3, I, I, I);
   testScanfPartial("%d%d%d%d //4\n", 4, 4, I, I, I, I);
 
   testScanfPartial("%d%n%n%d //1\n", 1, 3, I, I, I);
   testScanfPartial("%d%n%n%d //2\n", 2, 4, I, I, I, I);
 
-  testScanfPartial("%d%n%n%d %s %s", 3, 5, I, I, I, I, scanf_buf_size);
-  testScanfPartial("%d%n%n%d %s %s", 4, 6, I, I, I, I, scanf_buf_size,
-                   scanf_buf_size);
+  testScanfPartial("%d%n%n%d %s %s", 3, 5, I, I, I, I, test_buf_size);
+  testScanfPartial("%d%n%n%d %s %s", 4, 6, I, I, I, I, test_buf_size,
+                   test_buf_size);
+}
+
+static void testPrintf3(void *ctx, const char *format, ...) {
+  va_list ap;
+  va_start(ap, format);
+  printf_common(ctx, format, ap);
+  va_end(ap);
+}
+
+static void testPrintf2(const char *format, unsigned n,
+                       va_list expected_sizes) {
+  std::vector<unsigned> printf_sizes;
+  // 16 args should be enough.
+  testPrintf3((void *)&printf_sizes, format,
+             test_buf, test_buf, test_buf, test_buf, test_buf, test_buf,
+             test_buf, test_buf, test_buf, test_buf, test_buf, test_buf,
+             test_buf, test_buf, test_buf, test_buf);
+  verifyFormatResults(format, n, printf_sizes, expected_sizes);
+}
+
+static void testPrintf(const char *format, unsigned n, ...) {
+  va_list ap;
+  va_start(ap, n);
+  testPrintf2(format, n, ap);
+  va_end(ap);
+}
+
+TEST(SanitizerCommonInterceptors, Printf) {
+  // Only test functionality which differs from scanf
+
+  // Indexed arguments
+  testPrintf("%5$d", 0);
+  testPrintf("%.*5$d", 0);
+
+  // errno
+  testPrintf("%0-m", 0);
+
+  // Dynamic width
+  testPrintf("%*n", 1, I);
+  testPrintf("%*.10n", 1, I);
+
+  // Precision
+  testPrintf("%10.10n", 1, I);
+
+  // Dynamic precision
+  testPrintf("%.*n", 1, I);
+  testPrintf("%10.*n", 1, I);
 }

lib/tsan/lit_tests/printf-1.c

+// RUN: %clang_tsan -O2 %s -o %t
+// RUN: ASAN_OPTIONS=check_printf=1 %t 2>&1 | FileCheck %s
+// RUN: ASAN_OPTIONS=check_printf=0 %t 2>&1 | FileCheck %s
+// RUN: ASAN_OPTIONS="" %t 2>&1 | FileCheck %s
+
+#include <stdio.h>
+int main() {
+  volatile char c = '0';
+  volatile int x = 12;
+  volatile float f = 1.239;
+  volatile char s[] = "34";
+  printf("%c %d %.3f %s\n", c, x, f, s);
+  return 0;
+  // Check that printf works fine under Tsan.
+  // CHECK: 0 12 1.239 34
+}

lib/tsan/rtl/tsan_stat.h

enum StatType {
   StatInt_sscanf,
   StatInt_fscanf,
   StatInt___isoc99_vscanf,
   StatInt___isoc99_vsscanf,
   StatInt___isoc99_vfscanf,
   StatInt___isoc99_scanf,
   StatInt___isoc99_sscanf,
   StatInt___isoc99_fscanf,
+  StatInt_vprintf,
+  StatInt_vsprintf,
+  StatInt_vsnprintf,
+  StatInt_vasprintf,
+  StatInt_vfprintf,
+  StatInt_printf,
+  StatInt_sprintf,
+  StatInt_snprintf,
+  StatInt_asprintf,
+  StatInt_fprintf,
+  StatInt___isoc99_vprintf,
+  StatInt___isoc99_vsprintf,
+  StatInt___isoc99_vsnprintf,
+  StatInt___isoc99_vasprintf,
+  StatInt___isoc99_vfprintf,
+  StatInt___isoc99_printf,
+  StatInt___isoc99_sprintf,
+  StatInt___isoc99_snprintf,
+  StatInt___isoc99_asprintf,
+  StatInt___isoc99_fprintf,
   StatInt_on_exit,
   StatInt___cxa_atexit,
   StatInt_localtime,
   StatInt_localtime_r,
   StatInt_gmtime,
   StatInt_gmtime_r,
   StatInt_ctime,
   StatInt_ctime_r,

lib/tsan/rtl/tsan_stat.cc

void StatOutput(u64 *stat) {
   name[StatInt_sscanf]                   = "  sscanf                          ";
   name[StatInt_fscanf]                   = "  fscanf                          ";
   name[StatInt___isoc99_vscanf]          = "  vscanf                          ";
   name[StatInt___isoc99_vsscanf]         = "  vsscanf                         ";
   name[StatInt___isoc99_vfscanf]         = "  vfscanf                         ";
   name[StatInt___isoc99_scanf]           = "  scanf                           ";
   name[StatInt___isoc99_sscanf]          = "  sscanf                          ";
   name[StatInt___isoc99_fscanf]          = "  fscanf                          ";
+  name[StatInt_vprintf]                  = "  vprintf                         ";
+  name[StatInt_vsprintf]                 = "  vsprintf                        ";
+  name[StatInt_vsnprintf]                = "  vsnprintf                       ";
+  name[StatInt_vasprintf]                = "  vasprintf                       ";
+  name[StatInt_vfprintf]                 = "  vfprintf                        ";
+  name[StatInt_printf]                   = "  printf                          ";
+  name[StatInt_sprintf]                  = "  sprintf                         ";
+  name[StatInt_snprintf]                 = "  snprintf                        ";
+  name[StatInt_asprintf]                 = "  asprintf                        ";
+  name[StatInt_fprintf]                  = "  fprintf                         ";
+  name[StatInt___isoc99_vprintf]         = "  vprintf                         ";
+  name[StatInt___isoc99_vsprintf]        = "  vsprintf                        ";
+  name[StatInt___isoc99_vsnprintf]       = "  vsnprintf                       ";
+  name[StatInt___isoc99_vfprintf]        = "  vfprintf                        ";
+  name[StatInt___isoc99_printf]          = "  printf                          ";
+  name[StatInt___isoc99_sprintf]         = "  sprintf                         ";
+  name[StatInt___isoc99_snprintf]        = "  snprintf                        ";
+  name[StatInt___isoc99_fprintf]         = "  fprintf                         ";
   name[StatInt_on_exit]                  = "  on_exit                         ";
   name[StatInt___cxa_atexit]             = "  __cxa_atexit                    ";
   name[StatInt_localtime]                = "  localtime                       ";
   name[StatInt_localtime_r]              = "  localtime_r                     ";
   name[StatInt_gmtime]                   = "  gmtime                          ";
   name[StatInt_gmtime_r]                 = "  gmtime_r                        ";
   name[StatInt_ctime]                    = "  ctime                           ";
   name[StatInt_ctime_r]                  = "  ctime_r                         ";