Hi DM and everybody.
I changed the line to:
strncpy(*buf, cacheBlock->getEntry(entry), size);
(*buf)[size]='\0';
But it didn't work. Still crashes.
I believe I found the reason though. As I assumed many months ago, the reading
process works like this:
decryption -> decompression -> display.
Proof (zstr.cpp):
unsigned long len = size;
buf.setSize(size);
rawZFilter(buf, 0); // 0 = decipher
compressor->zBuf(&len, buf.getRawData());
char *rawBuf = compressor->Buf(0, &len);
cacheBlock = new EntriesBlock(rawBuf, len);
cacheBlockIndex = block;
That means we're trying to decompress faulty data! Can we at least check for a
zip signature or something in the decompressor?
This must also be the reason for the frequent warnings
"no room in outbuffer to during decompression. see zipcomp.cpp".
If I have the "flow of events correct" this time, then we have a design
problem. It would be best to change the flow of events and rebuild the
modules and release a fixed and incompatible sword version.
Or am I wrong? Please advise.
Martin
Am Montag, 27. November 2006 22:51 schrieb DM Smith:
> Joachim Ansorg wrote:
> >> zStr::getCompressedText calls
> >> strcpy(*buf, cacheBlock->getEntry(entry));
> >
> > My fix for this would be (without digging deep into the sources) in line
> > 438 of zstr.cpp:
> > strncpy(*buf, cacheBlock->getEntry(entry), size);
>
> This takes me way back. I haven't done serious C coding in years! But I
> remember something about this one. I had been bitten by it several times.
>
> IIRC:
> strncpy will not null terminate the string. Generally its good to follow
> it with buf[size] = '\0';
>
> strncpy will stop copying if it sees '\0' before it reaches the end. In
> that case, strncpy stops after it copies the '\0'.
> It may be good to compare strlen(buf) == size and throw an error if it
> is not.
>
> Putting this together:
> strncpy(*buf, cacheBlock->getEntry(entry), size);
> buf[size] = '\0';
> if (strlen(buf) != size) an error has occurred.
>
> Also, since we expect that the string is copied in its entirety, it is
> probably better and faster to use memcpy. Memmove would also work but
> would be slightly slower than memcpy.
>
> > strcpy expects a \0-terminated string. If the deciphering with the wrong
> > key creates a char* without a proper \0 this would result in an address
> > out of bounds. So the fix is to make sure we just copy the number of
> > bytes which are available in the cacheBlock.
> > I did not yet think whether a \0 has to explicitely be set at the end of
> > *buf.
> >
> > Does this make sense?
> > Does somebody have the setup to test this?
> >
> > Thanks,
> > Joachim
>
> _______________________________________________
> sword-devel mailing list: sword-devel@crosswire.org
> http://www.crosswire.org/mailman/listinfo/sword-devel
> Instructions to unsubscribe/change your settings at above page
_______________________________________________
sword-devel mailing list: sword-devel@crosswire.org
http://www.crosswire.org/mailman/listinfo/sword-devel
Instructions to unsubscribe/change your settings at above page
