[Gllug] Bad C code

[Nix]

On 20 Jul 2010, John Edwards stated:

> On Mon, Jul 19, 2010 at 11:39:06PM +0100, Nix wrote:
> <snip> 
>> system() with composed strings is Bad. Avoid avoid avoid. (In fact, more
>> generally, system() is Bad in and of itself, and
>> fork()/execvp()/waitpid() is easy enough that there's no excuse for
>> using system() ever.)
>
> If you ever want to see some really bad C code using system then look
> at the Smoothwall firewall.

I do not want to see 'bad C code using system' and 'firewall' in the
same sentence.

>                             They converted a whole bunch of shell
> scripts to C using system()

*twitch*

>                             so that they could be run from CGI.

*TWITCH*

Why not just put a setuid /bin/sh in the CGI directory and be done with
it? :/

> The installpackage.c in version 2.0 had this lovely gem:
> --------
>         memset(command, 0, STRING_SIZE);

Yay. Nice statically-sized start. (calloc()? what is that?)

>         snprintf(command, STRING_SIZE - 1, "/bin/rm -rf	/var/patches/%s", argv[1]);

Nice blanks (possibly line-wrapping on your part?) Nice horrifying
command. I hope %s is not user-provided.

>         system(command);

AUGH!

> To be fair by this point they had added some sanity checks:
> --------
>         if (strspn(argv[1], NUMBERS) != strlen(argv[1]))

AUGH! (simply using strspn() is odd enough. I'm not sure I've ever seen
it used in *good* code, possibly because nobody ever remembers it
exists because its name is so bloody awful.)

> But these were the people who tried to rewrite the GPL to suit
> themselves.

So that they didn't have to embarrass themselves by letting anyone look
at their code? :/
-- 
Gllug mailing list  -  Gllug at gllug.org.uk
http://lists.gllug.org.uk/mailman/listinfo/gllug