After going too long without any tuits, I've gotten around to properly testing this. Looks ok, although I didn't really do anything in-depth. - I'm going to commit and roll another RC.
Issac Joe Schaefer wrote: > Joe Schaefer <[EMAIL PROTECTED]> writes: > > >> Issac Goldstand <[EMAIL PROTECTED]> writes: >> >> >>> The apreq developers are planning a maintenance release of >>> libapreq1. This version primarily addresses an issue noted >>> with FireFox 2.0 truncating file uploads in SSL mode. >>> >>> Please give the tarball at >>> >>> http://people.apache.org/~issac/libapreq-1.34-RC2.tar.gz >>> >>> a try and report comments/problems/etc. to the apreq-dev list >>> at [EMAIL PROTECTED] >>> >> +1, tested on Debian stable i386. >> > > Having looked over some recent literature on memory allocation > attacks, I'm changing my vote to -1. We need to fix the > crappy quadratic allocator in multipart_buffer_read_body. > > Here's a first stab at it- completely untested (I didn't even > bother trying to compile it). The strategy here though is to > allocate (more than) twice the amount last allocated, so the > total amount allocated will sum (as a geometric series) to > no more than 2 times the final allocation, which is O(n). > > The problem with the current code is that the total amount > allocated is O(n^2), where n is basically the number of packets > received. This is entirely unsafe nowadays, so we should not bless > a new release which contains such code. > > Index: c/apache_multipart_buffer.c > =================================================================== > --- c/apache_multipart_buffer.c (revision 531273) > +++ c/apache_multipart_buffer.c (working copy) > @@ -273,17 +273,25 @@ > return len; > } > > -/* > - XXX: this is horrible memory-usage-wise, but we only expect > - to do this on small pieces of form data. > -*/ > char *multipart_buffer_read_body(multipart_buffer *self) > { > char buf[FILLUNIT], *out = ""; > + size_t nalloc = 1, cur_len = 0; > > - while(multipart_buffer_read(self, buf, sizeof(buf))) > - out = ap_pstrcat(self->r->pool, out, buf, NULL); > + while(multipart_buffer_read(self, buf, sizeof(buf))) { > + size_t len = strlen(buf); > + if (len + cur_len + 1 > nalloc) { > + char *tmp; > + nalloc = 2 * (nalloc + len + 1); > + tmp = ap_palloc(self->r->pool, nalloc); > + strcpy(tmp, out); > + out = tmp; > + } > > + strcpy(out + cur_len, buf); > + cur_len += len; > + } > + > #ifdef DEBUG > ap_log_rerror(MPB_ERROR, "multipart_buffer_read_body: '%s'", out); > #endif > > >
