3 ms·
I love C, but it's pretty scary sometime. 5 minutes ago, "I wonder if I can find a potential memory overwrite in 5 minutes?" Sure enough, the function StrAppen
by geoffdunbar 5y ago
I love C, but it's pretty scary sometime. 5 minutes ago, "I wonder if I can find a potential memory overwrite in 5 minutes?"
Sure enough, the function StrAppend potentially overflows a size_t size (without checking), and then writes into memory could be past the end of the allocated buffer. Given 5 minutes, I didn't look thoroughly if this is actually exploitable, but it's definitely a red-flag for the code. Be careful out there! Hopefully I am missing something, or this is just a simple oversight, but I would carefully audit this code before using it.
Submitted a ticket through the Althttpd website.
static char StrAppend(char zPrior, const char zSep, const char zSrc){
char zDest;
size_t size;
size_t n0, n1, n2;
if( zSrc==0 ) return 0;
if( zPrior==0 ) return StrDup(zSrc);
n0 = strlen(zPrior);
n1 = strlen(zSep);
n2 = strlen(zSrc);
size = n0+n1+n2+1;
zDest = (char*)SafeMalloc( size );
memcpy(zDest, zPrior, n0);
free(zPrior);
memcpy(&zDest[n0],zSep,n1);
memcpy(&zDest[n0+n1],zSrc,n2+1);
return zDest;
}
- spacechild1 5y ago> Sure enough, the function StrAppend potentially overflows a size_t size How should this happen in practice? The three strings would have to be larger than the available address space...
- ectospheno 5y agoYeah. The function in question is called in only one place. It would seem you’d need to send the web server more than a size_t of data for this to be an issue.
- acqq 5y agoExactly. In a single file C nobody can expect to get universal library functions that work in any possible imaginable context. The only relevant context is the code the function is in. And in that context, the function is doing enough.
- geoffdunbar 5y agoYes, absolutely. If the webserver is compiled 32-bit, that is only 4GB of data, which might be feasible? I don't know enough to say. Assuming a hacker kindly won't overflow your buffer is never a good idea. However, the presence of one piece of code that is not integer-overflow safe definitely makes me nervous. This is just the one I found in 5 minutes, what else is in there?
- ectospheno 5y agoNone that stand out to me, including what you posted. Do you have a real example?
- acqq 5y agoIt's not an integer overflow that would be needed but an unsigned overflow. The way I see it, on 32-bits, that means that the size HTTP request would have to be bigger than what's available to both user application and the OS together. In short, one just can't get the input request that big. Of course, if you manage that, you'll disprove this claim.
- thisgoodlife 5y agoMAX_CONTENT_LENGTH is 250MB. You won’t be able to send 4GB of data.
- gallier2 5y agoIn most places it uses int for string and buffer sizes lengths. It wouldn't surprize me if 2GiB of data could trigger several overflows.
- jackewiehose 5y agoAnd there's only one call to StrAppend() which is easily verified as safe.