Between Brian, Paul and other mod_lua fans... can you please take a look at this and propose appropriate constraints? Sorry to remove the code, but the XXX: tag wasn't apparently enough of prod to resolve this flaw.
On 6/9/2010 10:02 PM, [email protected] wrote: > Author: wrowe > Date: Thu Jun 10 03:02:07 2010 > New Revision: 953203 > > URL: http://svn.apache.org/viewvc?rev=953203&view=rev > Log: > Drp ap_args_to_table due to missing constraints; a DoS waiting > for an exploit. > > Some mod_lua fan aught to revisit this and provide a sensible > implementation. > > Modified: > httpd/httpd/trunk/include/ap_mmn.h > httpd/httpd/trunk/include/util_script.h > httpd/httpd/trunk/modules/lua/lua_request.c > httpd/httpd/trunk/modules/lua/mod_lua.c > httpd/httpd/trunk/server/util_script.c > > Modified: httpd/httpd/trunk/include/ap_mmn.h > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/include/ap_mmn.h?rev=953203&r1=953202&r2=953203&view=diff > ============================================================================== > --- httpd/httpd/trunk/include/ap_mmn.h (original) > +++ httpd/httpd/trunk/include/ap_mmn.h Thu Jun 10 03:02:07 2010 > @@ -227,13 +227,13 @@ > * Introduce per-module loglevels > * 20100606.1 (2.3.6-dev) Added extended timestamp formatting via > * ap_recent_ctime_ex(). > - * > + * 20100609.0 (2.3.6-dev) Dropped ap_args_to_table due to missing > constraints. > */ > > #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */ > > #ifndef MODULE_MAGIC_NUMBER_MAJOR > -#define MODULE_MAGIC_NUMBER_MAJOR 20100606 > +#define MODULE_MAGIC_NUMBER_MAJOR 20100609 > #endif > #define MODULE_MAGIC_NUMBER_MINOR 0 /* 0...n */ > > > Modified: httpd/httpd/trunk/include/util_script.h > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/include/util_script.h?rev=953203&r1=953202&r2=953203&view=diff > ============================================================================== > --- httpd/httpd/trunk/include/util_script.h (original) > +++ httpd/httpd/trunk/include/util_script.h Thu Jun 10 03:02:07 2010 > @@ -142,8 +142,6 @@ AP_DECLARE(int) ap_scan_script_header_er > > AP_DECLARE(void) ap_args_to_table(request_rec *r, apr_table_t **table); > > -AP_DECLARE(apr_status_t) ap_body_to_table(request_rec *r, apr_table_t > **table); > - > #ifdef __cplusplus > } > #endif > > Modified: httpd/httpd/trunk/modules/lua/lua_request.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/lua/lua_request.c?rev=953203&r1=953202&r2=953203&view=diff > ============================================================================== > --- httpd/httpd/trunk/modules/lua/lua_request.c (original) > +++ httpd/httpd/trunk/modules/lua/lua_request.c Thu Jun 10 03:02:07 2010 > @@ -189,19 +189,6 @@ static int req_write(lua_State *L) > return 0; > } > > -/* r:parsebody() */ > -static int req_parsebody(lua_State *L) > -{ > - apr_table_t *form_table; > - request_rec *r = ap_lua_check_request_rec(L, 1); > - lua_newtable(L); > - lua_newtable(L); > - if (ap_body_to_table(r, &form_table) == APR_SUCCESS) { > - apr_table_do(req_aprtable2luatable_cb, L, form_table, NULL); > - } > - return 2; > -} > - > /* r:addoutputfilter(name|function) */ > static int req_add_output_filter(lua_State *L) > { > @@ -538,8 +525,6 @@ AP_LUA_DECLARE(void) ap_lua_load_request > makefun(&req_document_root, APL_REQ_FUNTYPE_STRING, p)); > apr_hash_set(dispatch, "parseargs", APR_HASH_KEY_STRING, > makefun(&req_parseargs, APL_REQ_FUNTYPE_LUACFUN, p)); > - apr_hash_set(dispatch, "parsebody", APR_HASH_KEY_STRING, > - makefun(&req_parsebody, APL_REQ_FUNTYPE_LUACFUN, p)); > apr_hash_set(dispatch, "debug", APR_HASH_KEY_STRING, > makefun(&req_debug, APL_REQ_FUNTYPE_LUACFUN, p)); > apr_hash_set(dispatch, "info", APR_HASH_KEY_STRING, > > Modified: httpd/httpd/trunk/modules/lua/mod_lua.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/lua/mod_lua.c?rev=953203&r1=953202&r2=953203&view=diff > ============================================================================== > --- httpd/httpd/trunk/modules/lua/mod_lua.c (original) > +++ httpd/httpd/trunk/modules/lua/mod_lua.c Thu Jun 10 03:02:07 2010 > @@ -373,7 +373,7 @@ static const char *direct_chunkreader(lu > > for (p = ctx->buf; isspace(*p); ++p); > if (p[0] == '<' && p[1] == '/') { > - int i = 0; > + apr_size_t i = 0; > while (i < strlen(ctx->endstr)) { > if (tolower(p[i + 2]) != ctx->endstr[i]) > return ctx->buf; > > Modified: httpd/httpd/trunk/server/util_script.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_script.c?rev=953203&r1=953202&r2=953203&view=diff > ============================================================================== > --- httpd/httpd/trunk/server/util_script.c (original) > +++ httpd/httpd/trunk/server/util_script.c Thu Jun 10 03:02:07 2010 > @@ -760,83 +760,3 @@ AP_DECLARE(void) ap_args_to_table(reques > argstr_to_table(apr_pstrdup(r->pool, r->args), t); > *table = t; > } > - > -AP_DECLARE(apr_status_t) ap_body_to_table(request_rec *r, apr_table_t > **table) > -{ > - apr_bucket_brigade *bb; > - apr_bucket_brigade *tmpbb; > - apr_status_t rv = APR_SUCCESS; > - > - if (r->body_table) { > - *table = r->body_table; > - return APR_SUCCESS; > - } > - > - *table = NULL; > - > - bb = apr_brigade_create(r->pool, r->connection->bucket_alloc); > - tmpbb = apr_brigade_create(r->pool, r->connection->bucket_alloc); > - > - do { > - apr_off_t len; > - > - rv = ap_get_brigade(r->input_filters, tmpbb, AP_MODE_READBYTES, > - APR_BLOCK_READ, AP_IOBUFSIZE); > - if (rv) { > - break; > - } > - > - rv = apr_brigade_length(tmpbb, 1, &len); > - if (rv) { > - break; > - } > - > - if (len == 0) { > - break; > - } > - > - APR_BRIGADE_CONCAT(bb, tmpbb); > - } while(1); > - > - if (!rv) { > - r->body_table = apr_table_make(r->pool, 10); > - > - if (!APR_BRIGADE_EMPTY(bb)) { > - char *buffer; > - apr_off_t len; > - apr_pool_t *tpool; > - > - apr_pool_create(&tpool, r->pool); > - > - rv = apr_brigade_length(bb, 1, &len); > - > - if (!rv) { > - apr_size_t total; > - /* XXX where's our test that len fits in memory??? > - * theoretically can be a large file > ram space. > - * need to cast len to apr_size_t but it would mask > - * this notable mistake > - */ > - buffer = apr_palloc(tpool, len+1); > - > - total = len+1; > - > - rv = apr_brigade_flatten(bb, buffer, &total); > - > - buffer[total] = '\0'; > - > - argstr_to_table(buffer, r->body_table); > - } > - apr_pool_destroy(tpool); > - } > - } > - > - apr_brigade_destroy(bb); > - apr_brigade_destroy(tmpbb); > - > - *table = r->body_table; > - > - return rv; > -} > - > - > > >
