[sword-devel] crashes with ciphering code

[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
>