Hi all,
At Firday's nights meeting I said that I had been looking at the
Ecartis Mailing List Manager
http://www.ecartis.org/
and had found some potentially dangerous uses of the standard C
sscanf() function using FlawFinder:
http://www.dwheeler.com/flawfinder/
I have now reviewed the situation more carefully and found that
the Ecartis use of sscanf() is in fact safe even though FlawFinder
flags it as a potential problem.
Ecarts has a number of calls like this:
sscanf (buffer1, "From : %s", buffer2) ;
where buffer1 has been read from a email received from the internet
and both buffers are allocated on the process stack.
The problem here is that the email is untrusted, and can potentially
contain a line of data which is longer than buffer2, resulting in
the program overflowing the process stack with copious mayhen following.
Investigating this a little further however and this is proved to be
quite benign. The larger picture looks like this (slimmed down for
clarity):
void
function (FILE *stream)
{ char buffer1 [BIG_BUF], buffer2 [BIG_BUF] ;
/* Zero fill the buffers. */
memset (buffer1, 0, sizeof (buffer1)) ;
memset (buffer1, 0, sizeof (buffer1)) ;
/* Read data from the file (ie email received from the net). */
if (fgets (buffer1, sizeof (buffer1), stream) != NULL)
{ sscanf (buffer1, "From: %s", buffer2) ;
/* Do more stuff */
}
return ;
} /* function */
So is this safe? Well,
1) fgets() is guaranteed to zero terminate the buffer and will not
overflow if the buffer size has been correctly specified.
2) sizeof (buffer1) == sizef (buffer2), so the sscanf is also
guaranteed to avoid overflowing its destination buffer.
This all goes to prove that checking tools like Flawfinder are useful,
but don't see the full picture. It also shows that I shouldn't go
around blabbing my head off until I have investigated the situation
properly.
Anyway, Ecartis is now looking like a really nice Mailing List Manager
although I still have to complete my code review of it. Flawfinder is
also a really nice little tool which can help a code reviewer zero
in on the potential problems which can then been examined more closely.
Cheers,
Erik
--
+-----------------------------------------------------------+
Erik de Castro Lopo [EMAIL PROTECTED] (Yes it's valid)
+-----------------------------------------------------------+
"I ran it on my DeathStation 9000 and demons flew out of my nose." --Kaz
--
SLUG - Sydney Linux User's Group - http://slug.org.au/
More Info: http://lists.slug.org.au/listinfo/slug
