Hi!
I think I have stumbeled upon quite serious bugs in libapreq-
0.31 - these might have been addressed already, but since I
didn't find a archive of this list (I'm new), I decided to
mail anyway.
The thing I noticed first was that when using a multipart form,
information could pass over between sessions - the buffers
where not cleared! I noticed this beacause I use a single apache
process while developing, but we were able to recreate the bug
in our 'production' environment. If one submits a quite long
text and then a shorter one, the second submit will include the
end of the first one.
While trying to fix this quickly I found other problems:
- libapreq segfaulted if the submitted text was exactly a
'multipart-buffer' long (5120)
- a submitted text could never be more than two multipart-buffers
long (2*5120)
I have included a patch to libapreq-0.31, but it's just a quick
fix (although it seems to work).
Comments?
[ marc englund | [EMAIL PROTECTED] | +358 40 8408483 | 97895957 ]
diff -Naur libapreq-0.31-orig/c/apache_request.c
libapreq-0.31/c/apache_request.c
--- libapreq-0.31-orig/c/apache_request.c Sat Jul 3 04:00:17 1999
+++ libapreq-0.31/c/apache_request.c Wed Jan 31 14:06:47 2001
@@ -205,7 +205,7 @@
return ApacheRequest_parse_urlencoded(req);
}
else if (ct && *ct == 'm' && strstr(ct, "multipart/form-data")) {
- return ApacheRequest_parse_multipart(req);
+ return ApacheRequest_parse_multipart(req);
}
else {
ap_log_rerror(REQ_ERROR,
@@ -402,6 +402,12 @@
}
if (!filename) {
char *value = multipart_buffer_read_body(mbuff);
+ /* strip trailing CRLF */
+ if (value[strlen(value)-1] == '\012' &&
+ value[strlen(value)-2] == '\015') {
+
+ value[strlen(value)-2] = '\0';
+ }
ap_table_add(req->parms, param, value);
continue;
}
diff -Naur libapreq-0.31-orig/c/multipart_buffer.c
libapreq-0.31/c/multipart_buffer.c
--- libapreq-0.31-orig/c/multipart_buffer.c Sat May 1 09:44:28 1999
+++ libapreq-0.31/c/multipart_buffer.c Wed Jan 31 14:06:39 2001
@@ -67,9 +67,12 @@
{
char *res;
int len = lenone+lentwo;
- res = (char *)ap_palloc(p, len + 1);
- memcpy(res, one, lenone);
+ res = (char *)ap_palloc(p, len + 1);
+ /* initialize allocated memory to empty string */
+ memset(res, '\0', len + 1);
+ memcpy(res, one, lenone);
memcpy(res+lenone, two, lentwo);
+
return res;
}
@@ -117,19 +120,21 @@
return;
}
length = (bytes - buffer_len) + self->boundary_length + 2;
+
if (self->length < length) {
length = self->length;
}
bytes_to_read = length;
-
+
ap_hard_timeout("multipart_buffer_fill", self->r);
while (bytes_to_read > 0) {
/*XXX: we can optimize this loop*/
char *buff = (char *)ap_pcalloc(self->subp,
sizeof(char) * bytes_to_read + 1);
+ /* initialize allocacted memory to empty string */
+ memset(buff, '\0', sizeof(char) * bytes_to_read + 1);
len_read = ap_get_client_block(self->r, buff, bytes_to_read);
-
- if (len_read < 0) {
+ if (len_read <= 0) {
ap_log_rerror(MPB_ERROR,
"[libapreq] client dropped connection during read");
self->length = 0;
@@ -223,16 +228,29 @@
}
retval = ap_pstrndup(self->r->pool, self->buffer, *blen);
-
+
self->buffer += *blen;
self->buffer_len -= *blen;
/* If we hit the boundary, remove the CRLF from the end. */
- if (start > 0) {
- *blen -= 2;
- retval[*blen] = '\0';
- }
+ if(start > 0 && *blen > 1 &&
+ retval[*blen-1] == '\012' &&
+ retval[*blen-2] == '\015') {
+
+ *blen -= 2;
+ retval[*blen] = '\0';
+
+ }
+ /*
+ if (start > 0 && *blen > 0 &&
+ retval[*blen] == '\012' &&
+ retval[*blen-1] == '\015' ){
+
+ *blen -= 1;
+ retval[*blen] = '\0';
+ }
+ */
return retval;
}
@@ -295,7 +313,7 @@
retval = retval ?
my_join(self->r->pool, retval, old_len, data, blen) :
ap_pstrndup(self->r->pool, data, blen);
- old_len = blen;
+ old_len += blen;
}
return retval;
