Daniel Shahaf wrote on Sun, Aug 18, 2013 at 16:30:45 +0300:
> Masaru Tsuchiyama wrote on Sun, Aug 18, 2013 at 22:13:54 +0900:
> > Daniel Shahaf wrote:
> >> I think that's the wrong fix. Input validation should be done by
> >> checking that the input is valid, not by ruling out every known invalid
> >> value.[1] In this case: by checking os.path.exists() at the point in the
> >> code that tries to interpret the argument as a filename.
> >
> > Fixed in the attached patch.
> >
> > Regards.
> >
> > --
> > Masaru Tsuchiyama <m.tma...@gmail.com>
>
> > Index: gen-make.py
> > ===================================================================
> > --- gen-make.py (revision 1515099)
> > +++ gen-make.py (working copy)
> > @@ -278,6 +278,9 @@ if __name__ == '__main__':
> > except getopt.GetoptError, e:
> > _usage_exit(str(e))
> >
> > + if args and args[0] and os.path.exists(args[0]) != True:
>
> You shouldn't compare to True or to False. Just use the value as
> a boolean:
>
> if args and args[0] and not os.path.exists(args[0]):
>
> > + _usage_exit("argument must be a path to build.conf file")
> > +
> > conf = 'build.conf'
> > skip = 0
> > gentype = 'make'
>
> This fix doesn't handle permission problems (which I mentioned in my
> previous mail). I think the easiest way to handle those (as well as any
> other, unforeseen problems that .read() might run into and sweep under
> the rug) is to open the file ourselves:
To further explain this consideration: even if both existence and
permissions were checked, the code would *still* be wrong if args[0]
were a directory. If an .isdir() check were added, the code would
still be wrong if args[0] were a dangling symlink. And so on.
Calling open() will detect all those potential problems.
