[PATCH] Sanitize printf functions
Yury Gribov
tetra2005 at gmail.com
Mon Dec 30 03:26:53 PST 2013
================
Comment at: sanitizer_common/sanitizer_common_interceptors_scanf.inc:281
@@ -280,1 +280,3 @@
+ // TODO(y.gribov): check format?
+
----------------
Alexander Potapenko wrote:
> Alexander Potapenko wrote:
> > Yury Gribov wrote:
> > > Alexander Potapenko wrote:
> > > > Yury Gribov wrote:
> > > > > Kostya Serebryany wrote:
> > > > > > good idea. feel free to implement or to leave the TODO.
> > > > > > Actually this code base uses FIXME (w/o name) more often.
> > > > > > 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.
> > > > 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);
> > > >
> > > > ?
> > > 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?
> > > 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.
> > The bigger question is whether we want to check size of input buffer in sscanf and friends.
>
> Yes, that's nice to have, too.
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...
================
Comment at: sanitizer_common/sanitizer_common_interceptors_scanf.inc:362
@@ +361,3 @@
+ default:
+ break; // TODO(ygribov): issue warning?
+ }
----------------
Alexander Potapenko wrote:
> Yury Gribov wrote:
> > Alexander Potapenko wrote:
> > > Two spaces before the comment, please.
> > Do you think we need a warning here? If yes, I'll add it, if not - I'll remove the comment alltogether.
> We can't get an invalid size from format_get_value_size(), so it's ok to have a CHECK here.
Will do.
http://llvm-reviews.chandlerc.com/D2480
More information about the llvm-commits
mailing list