sprintf() is unsafe since it may overflow the bounds
of its destination buffers. Remove the last of the
calls to it; all the logic has either been rewritten
to use snprintf() or other forms of string copying
such as strlcpy().
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
This is the big push to get rid of the last of the
unadorned dynamic arrays. Use ptr_vectors for things
like mail conferences etc.
Lots of incidental cleanup along the way.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
I think this is correct. The code, both before and
after, doesn't appear to NUL-terminate its output.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
This started with using bounded operations on strings,
and morphed to introducing a utility function to open
the USERS SQLite3 database and then a general cleanup.
This needs testing.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
sprintf() was being used to copy a string constant with
no formatting verbs; just use strlcpy() instead.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
Note that the calls to strncat() did not account for the
NUL terminating byte, and for very long queries could have
led to a buffer overrun.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
In the course of removing calls to realloc(), change
the menu parsing and use logic to use ptr_vector's
directly.
This also fixes some detects menu issues in parsing
and avoids e.g. writing to a bad pointer (or should;
of course it needs testing...).
Finally, free menu state on return from the menu_system
function. There was a comment here to do that, but it
didn't appear to be done.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
The poorly named `strncpy` was originally written to
copy data into fixed-sized, disk-resident data structures
in an early version of the research Unix kernel. Thus, it
has peculiar semantics: it takes source and destination
pointer arguments and a length and will *always* modify
exactly `length` bytes in the destination buffer. If
the length of the source (which is presumed to be a
NUL-terminated C-stylestring) is `length` or more chars
long, then the result will not be NUL terminated. If it
is less than `length` bytes long, then the result will be
padded with zeros up to `length`.
This is all well and good for storing a file name into a
fixed-width directory entry in 6th edition Unix, but it's
not useful as a general-purpose string utility.
Replaced with calls to strlcpy(), which always properly
terminates the destination but doesn't have the additional
zeroing behavior. Since the buffers that we're copying
into were allocated with malloz(), and thus are guaranteed
to be filled with zeros, we're not leaking data, but not
double-zeroing either.
A few other things were changed. Lengths of destination
buffers are now given via `sizeof` instead of manifest
constants. One call to `memcpy` took the length from the
size of the source argument, thus possibly writing beyond
the end of the destination buffer. Changed to a call to
strlcpy() with length the sizeof destination.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
In bluewave.c mostly. There are a few places left where sprintf()
is called directly; these should be recast in terms of a stralloc
or possibly strlcat.
One small whitespace change in www_files.c.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
Replace most remaining uses of sprintf() into a `buffer`
variable followed by realloc() and strcat() with direct
use of stralloc.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
Specifically, change the www_last10 HTML rendering logic
to use stralloc and strftime(). This eliminates a lot of
duplication.
It would be easier to test this with a unit test if the
logic of reading the last10 entries from a file were
separated from the HTML rendering logic. An area for
future enhancement.
Also start in on www_email.c, which is the last bastion
of significant realloc() use for page generation. An
explicit goal is to get rid of unsafe string handling
functions such as strcpy, strcat, sprintf, etc.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
strcat()'ing a string onto the result of file2str()
will result in a buffer overflow, since file2str()
only allocates enough memory to hold the contents of
the file (plus a NUL terminator). This happend in
`bluewave.c`.
Instead, use `file2stralloc` to read the contents of
that file into a stralloc, which we can stralloc_cats
onto without fear of overflow.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
Make `blog_load` return a ptr_vector which is
consumed by the code that uses blog entries.
Greatly clean up WWW page generation by using
stralloc and strftime and the ptr_vector
infrastructure.
Needs to be tested. :-)
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
The pointer vector maintaining `content` in `editor` already
keeps track of the number of lines and makes it available via
a call to `ptr_vector_len` (or one could look at th `len`
member of the ptr_vector struct...this is C, not some fancy
object oriented language with data hiding). Delete the `lines`
local variable and just use ptr_vector_len where necessary.
Sorry; I should have done that in the first sweep through that
code. My bad!
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
strcalloc_starts() should have tested the return value
of `memcmp` against 0 for equality. Fixed and added a
test case.
As an aside, one might wonder how bugs like that are
creeping into well-tested code imported from other
projects? The answer, specific to stralloc, is that
the original code was very specific to qmail, and used
a number of additional functions specific to qmail.
Rather than import half of qmail, the version imported
into Magicka has been reworked to, instead, use
standard C functions. The process of modifying the
code gave rise to the opportunity for bugs to creep in.
Now that a unit testing framework is in place, we can
test things in isolation more easily and hopefully
catch such things BEFORE they are published to the
master repository.
Signed-off-by: Dan Cross <patchdev@fat-dragon.org>
