>On Tue, 07 Jan 2014 13:56:48 +0100 >Juliusz Chroboczek <[email protected]> wrote:
> Please help with testing. I've tagged my current head as > polipo-20140107, and I've put a tarball and 32-bit Windows binary on > > http://www.pps.univ-paris-diderot.fr/~jch/software/files/polipo/ There are two problems with the above tarball. Number one: it does not incorporate the fix for CVE-2011-3596 (something about caching local responses, I'm not familiar with the details of the vulnerability). The patch for polipo-1.0.4.1 applies cleanly to the release candidate tarball so I am attaching it. Second: several years ago, polipo had a bug on x86_64 where it would randomly crash. The bug was eventually tracked down to the va_list (construct for passing variable argument lists) and the differences in how AMD's and Intell's chips handle the va_list. Intell would pass-by-value, while AMD would pass-by-reference. During execution, the va_list gets destroyed and this (and I forgot the details) crashes the program when a particular operation gets done on the now-destroyed list. The solution was to copy the va_list every time something would be done to it, like when something got called and the list passed as the argument. When this fix was introduced to polipo, quite a few of the sensitive functions were not covered by the fix (and were therefore bugs waiting to happen). At the time, I posted a patch that applied the fix to all sensitive functions but this mail didn't generate any response. I have now rediffed the patch for the new release candidate and am resubmiting it for inclusion. Otherwise, the new polipo passed the preliminary test on my machine. We'll see how it handles the workload. -- Svi moji e-mailovi su kriptografski potpisani. Proverite ih. All of my e-mails are cryptographically signed. Verify them. -- You don't need an AI for a robot uprising. Humans will do just fine.
commit 0e2b44af619e46e365971ea52b97457bc0778cd3 Author: Christopher Davis <[email protected]> Date: Mon Jan 11 18:55:41 2010 -0800 Try to read POST requests to local configuration interface correctly. diff --git a/client.c b/client.c index 18f1d72..2404c81 100644 --- a/client.c +++ b/client.c @@ -987,7 +987,6 @@ httpClientDiscardBody(HTTPConnectionPtr connection) connection->reqlen = 0; httpConnectionDestroyReqbuf(connection); } - connection->reqte = TE_UNKNOWN; if(connection->bodylen > 0) { httpSetTimeout(connection, clientTimeout); diff --git a/local.c b/local.c index 82106c4..f59ab63 100644 --- a/local.c +++ b/local.c @@ -67,8 +67,8 @@ httpLocalRequest(ObjectPtr object, int method, int from, int to, requestor, closure); if(method >= METHOD_POST) { + httpClientDiscardBody(requestor->connection); httpClientError(requestor, 405, internAtom("Method not allowed")); - requestor->connection->flags &= ~CONN_READER; return 1; } @@ -295,11 +295,47 @@ httpSpecialRequest(ObjectPtr object, int method, int from, int to, return 1; } +#define MAXBODY(c) (((c)->flags & CONN_BIGREQBUF)? bigBufferSize : CHUNK_SIZE) + +static void +writeContinue(HTTPConnectionPtr client) +{ + static char httpContinue[] = "HTTP/1.1 100 Continue\r\n\r\n"; + + /* don't bother writing continue if the post is already completed */ + if (client->reqlen - client->reqbegin < client->bodylen) { + do_stream(IO_WRITE, client->fd, 0, httpContinue, 25, + httpErrorNofinishStreamHandler, client); + } +} + +static int +validateRequest(HTTPConnectionPtr client) +{ + ObjectPtr object = client->request->object; + + if (disableConfiguration) { + abortObject(object, 403, internAtom("Action not allowed")); + } else if (client->bodylen > MAXBODY(client)) { + abortObject(object, 411, internAtom("POST too large")); + } else if (!matchUrl("/polipo/status", object) && + !matchUrl("/polipo/config", object)) { + abortObject(object, 404, internAtom("Not found")); + } else + return 0; + + httpClientDiscardBody(client); + notifyObject(object); + + return -1; +} + int httpSpecialSideRequest(ObjectPtr object, int method, int from, int to, HTTPRequestPtr requestor, void *closure) { HTTPConnectionPtr client = requestor->connection; + int waiting = 0; assert(client->request == requestor); @@ -309,13 +345,24 @@ httpSpecialSideRequest(ObjectPtr object, int method, int from, int to, return 1; } + if (requestor->flags & REQUEST_WAIT_CONTINUE) { + requestor->flags &= ~REQUEST_WAIT_CONTINUE; + waiting = 1; + } + + if (validateRequest(client) < 0) + return 1; + + if (waiting) + writeContinue(client); + return httpSpecialDoSide(requestor); } -int -httpSpecialDoSide(HTTPRequestPtr requestor) +static int +readFinished(HTTPConnectionPtr client) { - HTTPConnectionPtr client = requestor->connection; + HTTPRequestPtr request = client->request; if(client->reqlen - client->reqbegin >= client->bodylen) { AtomPtr data; @@ -325,19 +372,25 @@ httpSpecialDoSide(HTTPRequestPtr requestor) client->reqlen = 0; if(data == NULL) { do_log(L_ERROR, "Couldn't allocate data.\n"); - httpClientError(requestor, 500, + httpClientError(request, 500, internAtom("Couldn't allocate data")); return 1; } - httpSpecialDoSideFinish(data, requestor); + httpSpecialDoSideFinish(data, request); return 1; } - if(client->reqlen - client->reqbegin >= CHUNK_SIZE) { - httpClientError(requestor, 500, internAtom("POST too large")); - return 1; - } + return 0; +} +int +httpSpecialDoSide(HTTPRequestPtr requestor) +{ + HTTPConnectionPtr client = requestor->connection; + + if (readFinished(client)) + return 1; + if(client->reqbegin > 0 && client->reqlen > client->reqbegin) { memmove(client->reqbuf, client->reqbuf + client->reqbegin, client->reqlen - client->reqbegin); @@ -346,7 +399,7 @@ httpSpecialDoSide(HTTPRequestPtr requestor) client->reqbegin = 0; do_stream(IO_READ | IO_NOTNOW, client->fd, - client->reqlen, client->reqbuf, CHUNK_SIZE, + client->reqlen, client->reqbuf, MAXBODY(client), httpSpecialClientSideHandler, client); return 1; } @@ -358,36 +411,22 @@ httpSpecialClientSideHandler(int status, { HTTPConnectionPtr connection = srequest->data; HTTPRequestPtr request = connection->request; - int push; - if((request->object->flags & OBJECT_ABORTED) || - !(request->object->flags & OBJECT_INPROGRESS)) { - httpClientDiscardBody(connection); - httpClientError(request, 503, internAtom("Post aborted")); - return 1; - } - - if(status < 0) { - do_log_error(L_ERROR, -status, "Reading from client"); - if(status == -EDOGRACEFUL) - httpClientFinish(connection, 1); - else - httpClientFinish(connection, 2); + if(status) { + connection->flags &= ~CONN_READER; + if (request->chandler) { + unregisterConditionHandler(request->chandler); + request->chandler = NULL; + } + do_log(L_ERROR, "Incomplete client request.\n"); + httpClientRawError(connection, 502, + internAtom("Incomplete client request"), 1); return 1; } - push = MIN(srequest->offset - connection->reqlen, - connection->bodylen - connection->reqoffset); - if(push > 0) { - connection->reqlen += push; - httpSpecialDoSide(request); - } + connection->reqlen = srequest->offset; - do_log(L_ERROR, "Incomplete client request.\n"); - connection->flags &= ~CONN_READER; - httpClientRawError(connection, 502, - internAtom("Incomplete client request"), 1); - return 1; + return readFinished(connection); } int @@ -480,7 +519,7 @@ httpSpecialDoSideFinish(AtomPtr data, HTTPRequestPtr requestor) object->flags &= ~OBJECT_INITIAL; object->length = 0; } else { - abortObject(object, 405, internAtom("Method not allowed")); + abortObject(object, 404, internAtom("Not found")); } out:
diff -Naur polipo-20140107-old/atom.c polipo-20140107-new/atom.c
--- polipo-20140107-old/atom.c 2014-01-07 13:33:19.000000000 +0100
+++ polipo-20140107-new/atom.c 2014-01-08 16:15:32.000000000 +0100
@@ -244,9 +244,12 @@
AtomPtr atom;
char *s1, *s2;
int n, rc;
+ va_list args_copy;
if(f) {
- s1 = vsprintf_a(f, args);
+ va_copy(args_copy, args);
+ s1 = vsprintf_a(f, args_copy);
+ va_end(args_copy);
if(s1 == NULL)
return NULL;
n = strlen(s1);
diff -Naur polipo-20140107-old/log.c polipo-20140107-new/log.c
--- polipo-20140107-old/log.c 2014-01-07 13:33:19.000000000 +0100
+++ polipo-20140107-new/log.c 2014-01-08 16:04:15.000000000 +0100
@@ -406,17 +406,21 @@
void
really_do_log_v(int type, const char *f, va_list args)
{
+ va_list args_copy, args_copy2;
+
if(type & LOGGING_MAX & logLevel) {
if(logF)
{
- va_list args_copy;
va_copy(args_copy, args);
vfprintf(logF, f, args_copy);
va_end(args_copy);
}
#ifdef HAVE_SYSLOG
- if(logSyslog)
- accumulateSyslogV(type, f, args);
+ if(logSyslog) {
+ va_copy(args_copy2, args);
+ accumulateSyslogV(type, f, args_copy2);
+ va_end(args_copy2);
+ }
#endif
}
}
@@ -434,13 +438,14 @@
void
really_do_log_error_v(int type, int e, const char *f, va_list args)
{
+ va_list args_copy, args_copy2;
+
if((type & LOGGING_MAX & logLevel) != 0) {
char *es = pstrerror(e);
if(es == NULL)
es = "Unknown error";
if(logF) {
- va_list args_copy;
va_copy(args_copy, args);
vfprintf(logF, f, args_copy);
fprintf(logF, ": %s\n", es);
@@ -451,7 +456,9 @@
char msg[256];
int n = 0;
- n = snnvprintf(msg, n, 256, f, args);
+ va_copy(args_copy2, args);
+ n = snnvprintf(msg, n, 256, f, args_copy2);
+ va_end(args_copy2);
n = snnprintf(msg, n, 256, ": ");
n = snnprint_n(msg, n, 256, es, strlen (es));
n = snnprintf(msg, n, 256, "\n");
diff -Naur polipo-20140107-old/util.c polipo-20140107-new/util.c
--- polipo-20140107-old/util.c 2014-01-07 13:33:19.000000000 +0100
+++ polipo-20140107-new/util.c 2014-01-08 16:11:36.000000000 +0100
@@ -50,9 +50,14 @@
snnvprintf(char *restrict buf, int n, int len, const char *format, va_list args)
{
int rc = -1;
+ va_list args_copy;
+
if(n < 0) return -2;
- if(n < len)
- rc = vsnprintf(buf + n, len - n, format, args);
+ if(n < len) {
+ va_copy(args_copy, args);
+ rc = vsnprintf(buf + n, len - n, format, args_copy);
+ va_end(args_copy);
+ }
if(rc >= 0 && n + rc <= len)
return n + rc;
else
@@ -268,8 +273,11 @@
{
char *r;
int rc;
+ va_list args_copy;
- rc = vasprintf(&r, f, args);
+ va_copy(args_copy, args);
+ rc = vasprintf(&r, f, args_copy);
+ va_end(args_copy);
if(rc < 0)
return NULL;
return r;
@@ -284,10 +292,11 @@
int n, size;
char buf[64];
char *string;
- va_list args_copy;
+ va_list args_copy, args_copy2, args_revolving;
va_copy(args_copy, args);
n = vsnprintf(buf, 64, f, args_copy);
+ va_end(args_copy);
if(n >= 0 && n < 64) {
return strdup_n(buf, n);
}
@@ -296,21 +305,28 @@
else
size = 96;
+ va_copy(args_revolving, args);
while(1) {
string = malloc(size);
- if(!string)
+ if(!string) {
+ va_end(args_revolving);
return NULL;
- va_copy(args_copy, args);
- n = vsnprintf(string, size, f, args_copy);
- if(n >= 0 && n < size)
+ }
+ va_copy(args_copy2, args_revolving);
+ n = vsnprintf(string, size, f, args_copy2);
+ va_end(args_copy2);
+ if(n >= 0 && n < size) {
+ va_end(args_revolving);
return string;
- else if(n >= size)
+ } else if(n >= size)
size = n + 1;
else
size = size * 3 / 2;
free(string);
- if(size > 16 * 1024)
+ if(size > 16 * 1024) {
+ va_end(args_revoling);
return NULL;
+ }
}
/* NOTREACHED */
}
signature.asc
Description: PGP signature
------------------------------------------------------------------------------ Rapidly troubleshoot problems before they affect your business. Most IT organizations don't have a clear picture of how application performance affects their revenue. With AppDynamics, you get 100% visibility into your Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro! http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
_______________________________________________ Polipo-users mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/polipo-users
