On Wed, 2011-01-05 at 20:27 +0100, Karel Zak wrote:
> On Wed, Jan 05, 2011 at 01:40:59PM -0300, Davidlohr Bueso wrote:
> > The tmpnam(3) function presents a security hazard, so use mkstemp(3) instead.
> > Even though this is a deprecated command, due to security reasons, we should address this problem.
>
> The perfect is the enemy of the good. I think it's better to have
> imperfect, but tested code...
>
> > @@ -135,12 +137,21 @@ int main (int argc, char **argv)
> > else command->name[0] = '\0';
> > break;
> > case COMMAND_DUMP_LIST:
> > - if (tmpnam (command->name) == NULL)
> > - {
> > + tmp = malloc(sizeof(char) * (strlen(command->name) + 8 ));
> > + if (!tmp)
> > + errx(EXIT_FAILURE, "cannot allocate memory\n");
>
> do you remember xalloc()? :-)
>
Since this is a deprecated function I didn't want to bother adding more
dependencies to the makefile.
> Anyway, sizeof(char) is unnecessary.
Yeah but ...old habits.
>
> > + sprintf(tmp, "%s-XXXXXX", command->name);
> > + if (-1 == (fd = mkstemp(tmp))) {
> > fprintf (stderr, "Unable to create a unique filename\t%s\n",
> > ERRSTRING);
> > exit (1);
> > }
> > + /* we don't use this file really */
> > + close(fd);
> > + unlink(tmp);
> > + free(tmp);
>
> Sorry, but I don't understand this change at all. The temporary file
> in the original code was used for the fifo. It seems that in your code
> the command->name is uninitialized and your tmp file is unused. Right?
>
The way I see it, the original code doesn't use the string created by
tmpnam (from the manpage I assume that it doesn't create the file,
unlike mkstemp, but only returns the file's name):
if (tmpnam (command->name) == NULL)
{
fprintf (stderr, "Unable to create a unique filename\t%s
\n",
ERRSTRING);
exit (1);
}
So the fifo does not use this. This is why I left the new tmp variable
unused and deleted the file. If this is correct, we should just actually
remove the code, but didn't want to go there :)
> Karel
>
> > +
> > if (mkfifo (command->name, S_IRUSR) != 0)
> > {
> > fprintf (stderr, "Unable to create FIFO: \"%s\"\t%s\n",
> > --
> > 1.7.1
> >
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe util-linux" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
