src/store_swapmeta.c / storeSwapMetaUnpack bug?

While playing with LUSCA and Valgrind i notice out of bounds memory access on
it. Most probably bug exists in squid 2.7 also.

Suspicious code is in src/store_swapmeta.c function storeSwapMetaUnpack
On power failure (or SCSI controller failure) i had file corruption, and i
don't have strong file integrity control (tradeoff for performance), so data
become corrupted.

As result, for example, i have file, where STORE_META_OK, but next field
(header length?) is invalid.
Let's say i have call
storeSwapMetaUnpack(const char *buf, int *hdr_len)
hdr_len is 1024, but at
    xmemcpy(&buflen, &buf[j], sizeof(int));
it will read invalid value, let's say 16384.

As result in loop below i will have out of bounds access to buf variable:
   while (buflen - j >= (sizeof(char) + sizeof(int))) {
        type = buf[j++];
        /* VOID is reserved, but allow some slack for new types.. */

Valgrind started to scream at " type = buf[j++]; "

Probably if i will check buflen for sanity (it should be less than *hdr_len -
sizeof(char) - sizeof(int)) it will help to avoid such problem...

Is my assumptions correct? Do you want me to open bug in bugzilla?
I repeat, i notice this bug at LUSCA, but code looks almost the same in
squid...

Please CC me, i am not subscribed on list.
Received on Wed Sep 30 2009 - 17:47:18 MDT