On Tue, 2006-04-25 at 17:26, Dave Miner wrote:
> Peter Tribble wrote:
> > On Mon, 2006-04-24 at 20:50, Dave Miner wrote:
> >>>>> 5. 6322837
> >>>>>
> >>>>> I've added a simple check that the input file specified is indeed a
> >>>>> regular file.
> >>>>>
[ ... skip ...]
> The problem is we know the obviously bad cases, but we don't necessarily 
> know all the good cases that may have been allowed by the existing code. 
>   So I tend to focus on doing what we know is correct, rather than 
> guessing on what might not cause a problem.

OK, I've ended up with:

        if (strcmp(file, "-") == 0) {
                fplist = stdin;
        } else {
                if ((fd = open(file, O_RDONLY)) == -1) {
                        progerr(gettext(ERR_IOPEN), file);
                        exit(1);
                }
                if (fstat(fd, &st) == -1) {
                        progerr(gettext(ERR_IOPEN), file);
                        exit(1);
                }
                if (S_ISDIR(st.st_mode) || S_ISBLK(st.st_mode)) {
                        progerr(gettext(ERR_PATHS_INVALID), file);
                        exit(1);
                }
                if ((fplist = fdopen(fd, "r")) == NULL) {
                        progerr(gettext(ERR_IOPEN), file);
                        exit(1);
                }
                if (S_ISREG(st.st_mode)) {
                        if (fgets(pathname, 5, fplist) == NULL) {
                                progerr(gettext(ERR_IEMPTY));
                                exit(1);
                        }
                        if (strcmp(pathname, "\177ELF") == 0) {
                                progerr(gettext(ERR_PATHS_INVALID), file);
                                exit(1);
                        }
                        rewind(fplist);
                }
        }

This blocks obviously wrong, and only bothers to do the ELF
check on regular files. The snag with this code in general
is that anything that gets past the checks will get caught
by the fscanf limit, so it shouldn't crash, but doesn't
give any useful diagnostics either.

New version at http://www.petertribble.co.uk/Solaris/fixes/1/

> > (I just had a horrid thought about hacking it so that if
> > it was a directory, it opened the directory and generated
> > a directory listing of files to check - perhaps we had
> > better make a decision before I start thinking any more.)
> >  
> 
> Perhaps an interesting feature to add, but that's another RFE we could 
> handle later.  Don't stop thinking ;-)

The more I think about this, the more I like it. I'll probably put
in an RFE in any case.

However, what I really need to do is direct my thinking back
towards performance improvements and fixes, rather than neat
tricks...

-- 
-Peter Tribble
L.I.S., University of Hertfordshire - http://www.herts.ac.uk/
http://www.petertribble.co.uk/ - http://ptribble.blogspot.com/



Reply via email to