Bug#533285: inn2: nnrpd crashes when retrieving entire thread

[Russ Allbery]

Julien ÉLIE <julien at trigofacile.com> writes:

> We currently have:
>
> char *
> overview_getheader(const struct cvector *vector, unsigned int element,
>     const struct vector *extra)
>
> We then just have to add a "const char *field".  But couldn't we keep
> the element number?  (Otherwise, we would also have to walk the
> mandatory fields -- and to change "Lines" and "Bytes" to respectively
> ":lines" and ":bytes".)

I think we want to change the element number to a const char *field and
then move the call to overview_index() into overview_getheader().  Then,
if the index number is within the mandatory headers, do what we do now,
and if it's outside, walk the fields looking with strncasecmp for a
header starting with the desired field followed by a colon and a space.

I suspect :bytes and :lines are already handled somewhere else, since I
don't see any code to handle them in overview_index(), but code could
easily be added there if need be.

>> xstrndup() got a perfectly valid size.  It's a very large size, but
>> there wasn't anything invalid about it.  The API takes a size_t,
>> which is unsigned.

> All right, it is true that the log says "failed to strndup 4294967278
> bytes" (so the signed int was cast to an unsigned int).

Right.  That happens because of the function signature and hence before
xstrndup ever sees the call.

The other problem is that there's no way for xstrndup to return an
error, since the point of the function is that it never returns NULL.
It could die with a different error message, but there doesn't seem to
be much point.  The underlying C library function strndup similarly
wouldn't check size_t.

I suppose the one sanity check that one could add is to strlen the
duplicated string and cap the size of the allocated string to the length
of the original instead of blindly trusting the size_t argument.
However, I suspect that change would only affect places where the caller
was already doing something fairly wrong, and there it would probably
just paper over a deeper bug (as it would have here).

-- 
Russ Allbery (rra at stanford.edu)             <http://www.eyrie.org/~eagle/>

    Please send questions to the list rather than mailing me directly.
     <http://www.eyrie.org/~eagle/faqs/questions.html> explains why.