Peter Tribble wrote:
> 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/
>
OK, I'm happy enough with this version, so we'll move ahead with it.
>>> (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.
>
I think the feature is reasonable, though I'm not sure I'd like
overloading it onto -i, as we've done things like that on other tools to
bad effect; too bad -d got used for a lower-value option already. Seems
like you'd also want a "-r" or something to specify recursive descent on
the directory.
Dave
