On Mon, Feb 11, 2013 at 04:00:50PM +0400, Evgeniy Stepanov wrote:
> > I've tested various glibc *scanf testcases with the GCC libsanitizer
> > + enabling of # define SANITIZER_INTERCEPT_SCANF SI_NOT_WINDOWS
> > + llvm svn diff between last merge point to GCC and current llvm trunk
> > + this patch, and it looked good.
>
> Sounds great. I'll commit the patch to compiler-rt if you are ok with that.
Yes, that is fine, thanks.
> > There is one further issue, I'd say you need to pass down to scanf_common
> > the result from the intercepted call, and use it to see if it is valid
> > to call strlen on the ptr, or if just 1 should be assumed for SSS_STRLEN.
> > Note 'n' doesn't count towards that. Because, it is unsafe to call strlen
> > on arguments that haven't been assigned yet. Testcase that still fails:
>
> Definitely. Also, I'm undecided on this, but we probably don't want to
> report access ranges for the arguments that were not written to. On
> the other hand, those are potential writes that did not happen due to,
> in most cases, user input - we might want to report them anyway.
Yeah. If used with say sscanf, the user might in theory know that
the extra arguments won't be written to, on the other side, it would be
bad programming practice in that case and potential problem if the string
passed is changed. But, using strlen is certainly wrong.
> > Oh, one more thing, on Linux for recent enough glibc's it would be
> > desirable to also intercept:
> > __isoc99_sscanf, __isoc99_scanf, __isoc99_vsscanf, __isoc99_fscanf,
> > __isoc99_vfscanf, __isoc99_vscanf
> > functions (guard the interception with
> > #if defined __GLIBC_PREREQ && __GLIBC_PREREQ (2, 7)
> > ). All these work exactly like the corresponding non-__isoc99_
> > functions, just %a followed by s, S or [ always writes float, thus
> > there is no ambiguity.
Note, these calls are used even in -std=c99 compiled code if -D_GNU_SOURCE
isn't used, so it might appear pretty often in various apps.
Jakub
