From a47a4ce4f962b7a225075669d203c82ea1a1b8cf Mon Sep 17 00:00:00 2001 From: Greg Wroblewski Date: Sat, 23 Mar 2013 23:01:26 -0700 Subject: [PATCH] Fixed two crashing bugs: race condition when module was initialized and failing config commands in libapr. --- iis/mymodulefactory.h | 11 +- standalone/config.c | 9 +- standalone/main.cpp | 252 +++++++++++++++++------------ standalone/standalone.vcxproj.user | 2 +- 4 files changed, 169 insertions(+), 105 deletions(-) diff --git a/iis/mymodulefactory.h b/iis/mymodulefactory.h index a073a78e..9934ab61 100644 --- a/iis/mymodulefactory.h +++ b/iis/mymodulefactory.h @@ -20,14 +20,17 @@ // of CMyHttpModule for each request. class CMyHttpModuleFactory : public IHttpModuleFactory { - CMyHttpModule * m_pModule; + CMyHttpModule * m_pModule; + CRITICAL_SECTION m_csLock; public: CMyHttpModuleFactory() { m_pModule = NULL; + + InitializeCriticalSection(&m_csLock); } - + virtual HRESULT GetHttpModule( @@ -43,6 +46,8 @@ public: goto Finished; } + EnterCriticalSection(&m_csLock); + if(m_pModule == NULL) { m_pModule = new CMyHttpModule(); @@ -54,6 +59,8 @@ public: } } + LeaveCriticalSection(&m_csLock); + *ppModule = m_pModule; Finished: diff --git a/standalone/config.c b/standalone/config.c index b1a57dad..ec1c40f4 100644 --- a/standalone/config.c +++ b/standalone/config.c @@ -1153,7 +1153,14 @@ ProcessInclude: parms->directive = newdir; - errmsg = invoke_cmd(cmd, parms, mconfig, args); + __try + { + errmsg = invoke_cmd(cmd, parms, mconfig, args); + } + __except(EXCEPTION_EXECUTE_HANDLER) + { + errmsg = "Command failed to execute (check file/folder permissions, syntax, etc.)."; + } if(errmsg != NULL) break; diff --git a/standalone/main.cpp b/standalone/main.cpp index 73265b1a..912b96f9 100644 --- a/standalone/main.cpp +++ b/standalone/main.cpp @@ -21,6 +21,7 @@ char *config_file = NULL; +char *url_file = NULL; char *event_files[1024]; int event_file_cnt; char *event_file = NULL; @@ -31,6 +32,11 @@ int event_file_blocks[256]; #define EVENT_FILE_MAX_SIZE (16*1024*1024) +#define MAX_URLS 4096 + +char urls[MAX_URLS][4096]; +int url_cnt = 0; + void readeventfile(char *name) { if(event_file == NULL) @@ -119,6 +125,12 @@ void parseargs(int argc, char *argv[]) i += 2; continue; } + if(argv[i][1] == 'u' && i < argc - 1) + { + url_file = argv[i + 1]; + i += 2; + continue; + } i++; continue; } @@ -224,7 +236,7 @@ void main(int argc, char *argv[]) if(config_file == NULL || argc < 3) { printf("Usage:\n"); - printf("standalone.exe -c [ ...]\n"); + printf("standalone.exe -c [-u ] [ ...]\n"); return; } @@ -250,133 +262,171 @@ void main(int argc, char *argv[]) modsecInitProcess(); + if(url_file != NULL) + { + FILE *fr = fopen(url_file, "rb"); + int i = 0; + + while(fgets(urls[i],4096,fr) != NULL) + { + urls[i][4095] = 0; + + int l = strlen(urls[i]) - 1; + + if(l < 8) + continue; + + while(urls[i][l] == 10 || urls[i][l] == 13) + l--; + + urls[i++][l + 1] = 0; + } + + url_cnt = i; + fclose(fr); + } + for(int i = 0; i < event_file_cnt; i++) { - readeventfile(event_files[i]); - parseeventfile(); + if(url_cnt == 0) + { + urls[0][0] = 0; + url_cnt = 1; + } - bodypos = 0; - responsepos = 0; + for(int ui = 0; ui < url_cnt; ui++) + { + readeventfile(event_files[i]); + parseeventfile(); - c = modsecNewConnection(); + bodypos = 0; + responsepos = 0; - modsecProcessConnection(c); + c = modsecNewConnection(); - r = modsecNewRequest(c, config); + modsecProcessConnection(c); - int j = event_file_blocks['B']; + r = modsecNewRequest(c, config); - if(j < 0) - continue; + int j = event_file_blocks['B']; - j++; + if(j < 0) + continue; - if(event_file_lines[j][0] == 0) - continue; + j++; - char *method = event_file_lines[j]; - char *url = strchr(method, 32); - char *proto = strchr(url + 1, 32); + if(event_file_lines[j][0] == 0) + continue; - if(url == NULL || proto == NULL) - continue; + char *method = event_file_lines[j]; + char *url = strchr(method, 32); + char *proto = strchr(url + 1, 32); - *url++=0; - *proto++=0; + if(url == NULL || proto == NULL) + continue; + + *url++=0; + *proto++=0; + + if(urls[ui][0] != 0) + { + url = urls[ui]; + } #define SETMETHOD(m) if(strcmp(method,#m) == 0){ r->method = method; r->method_number = M_##m; } - r->method = "INVALID"; - r->method_number = M_INVALID; + r->method = "INVALID"; + r->method_number = M_INVALID; - SETMETHOD(OPTIONS) - SETMETHOD(GET) - SETMETHOD(POST) - SETMETHOD(PUT) - SETMETHOD(DELETE) - SETMETHOD(TRACE) - SETMETHOD(CONNECT) - SETMETHOD(MOVE) - SETMETHOD(COPY) - SETMETHOD(PROPFIND) - SETMETHOD(PROPPATCH) - SETMETHOD(MKCOL) - SETMETHOD(LOCK) - SETMETHOD(UNLOCK) + SETMETHOD(OPTIONS) + SETMETHOD(GET) + SETMETHOD(POST) + SETMETHOD(PUT) + SETMETHOD(DELETE) + SETMETHOD(TRACE) + SETMETHOD(CONNECT) + SETMETHOD(MOVE) + SETMETHOD(COPY) + SETMETHOD(PROPFIND) + SETMETHOD(PROPPATCH) + SETMETHOD(MKCOL) + SETMETHOD(LOCK) + SETMETHOD(UNLOCK) - r->protocol = proto; + r->protocol = proto; - while(event_file_lines[++j][0] != 0) - { - char *value = strchr(event_file_lines[j], ':'); + while(event_file_lines[++j][0] != 0) + { + char *value = strchr(event_file_lines[j], ':'); - if(value == NULL) - break; + if(value == NULL) + break; - *value++ = 0; + *value++ = 0; - while(*value <=32 && *value != 0) - value++; + while(*value <=32 && *value != 0) + value++; - apr_table_setn(r->headers_in, event_file_lines[j], value); - } + apr_table_setn(r->headers_in, event_file_lines[j], value); + } - r->content_encoding = apr_table_get(r->headers_in, "Content-Encoding"); - r->content_type = apr_table_get(r->headers_in, "Content-Type"); - r->hostname = apr_table_get(r->headers_in, "Host"); - r->path_info = url; + r->content_encoding = apr_table_get(r->headers_in, "Content-Encoding"); + r->content_type = apr_table_get(r->headers_in, "Content-Type"); + r->hostname = apr_table_get(r->headers_in, "Host"); + r->path_info = url; - char *query = strchr(url, '?'); - char *rawurl = url; + char *query = strchr(url, '?'); + char *rawurl = url; - if(query != NULL) - { - rawurl = (char *)apr_palloc(r->pool, strlen(url) + 1); - strcpy(rawurl, url); - *query++ = 0; - r->args = query; + if(query != NULL) + { + rawurl = (char *)apr_palloc(r->pool, strlen(url) + 1); + strcpy(rawurl, url); + *query++ = 0; + r->args = query; + } + + const char *lng = apr_table_get(r->headers_in, "Content-Languages"); + + if(lng != NULL) + { + r->content_languages = apr_array_make(r->pool, 1, sizeof(const char *)); + + *(const char **)apr_array_push(r->content_languages) = lng; + } + + r->request_time = apr_time_now(); + + r->parsed_uri.scheme = "http"; + r->parsed_uri.path = r->path_info; + r->parsed_uri.hostname = (char *)r->hostname; + r->parsed_uri.is_initialized = 1; + r->parsed_uri.port = 80; + r->parsed_uri.port_str = "80"; + r->parsed_uri.query = r->args; + r->parsed_uri.dns_looked_up = 0; + r->parsed_uri.dns_resolved = 0; + r->parsed_uri.password = NULL; + r->parsed_uri.user = NULL; + r->parsed_uri.fragment = NULL; + + r->unparsed_uri = rawurl; + r->uri = r->unparsed_uri; + + r->the_request = (char *)apr_palloc(r->pool, strlen(r->method) + 1 + strlen(r->uri) + 1 + strlen(r->protocol) + 1); + + strcpy(r->the_request, r->method); + strcat(r->the_request, " "); + strcat(r->the_request, r->uri); + strcat(r->the_request, " "); + strcat(r->the_request, r->protocol); + + apr_table_setn(r->subprocess_env, "UNIQUE_ID", "1"); + + modsecProcessRequest(r); + modsecProcessResponse(r); + modsecFinishRequest(r); } - - const char *lng = apr_table_get(r->headers_in, "Content-Languages"); - - if(lng != NULL) - { - r->content_languages = apr_array_make(r->pool, 1, sizeof(const char *)); - - *(const char **)apr_array_push(r->content_languages) = lng; - } - - r->request_time = apr_time_now(); - - r->parsed_uri.scheme = "http"; - r->parsed_uri.path = r->path_info; - r->parsed_uri.hostname = (char *)r->hostname; - r->parsed_uri.is_initialized = 1; - r->parsed_uri.port = 80; - r->parsed_uri.port_str = "80"; - r->parsed_uri.query = r->args; - r->parsed_uri.dns_looked_up = 0; - r->parsed_uri.dns_resolved = 0; - r->parsed_uri.password = NULL; - r->parsed_uri.user = NULL; - r->parsed_uri.fragment = NULL; - - r->unparsed_uri = rawurl; - r->uri = r->unparsed_uri; - - r->the_request = (char *)apr_palloc(r->pool, strlen(r->method) + 1 + strlen(r->uri) + 1 + strlen(r->protocol) + 1); - - strcpy(r->the_request, r->method); - strcat(r->the_request, " "); - strcat(r->the_request, r->uri); - strcat(r->the_request, " "); - strcat(r->the_request, r->protocol); - - apr_table_setn(r->subprocess_env, "UNIQUE_ID", "1"); - - modsecProcessRequest(r); - modsecProcessResponse(r); - modsecFinishRequest(r); } modsecTerminate(); diff --git a/standalone/standalone.vcxproj.user b/standalone/standalone.vcxproj.user index 0be15d4d..deb3e354 100644 --- a/standalone/standalone.vcxproj.user +++ b/standalone/standalone.vcxproj.user @@ -1,7 +1,7 @@  - -c owasp_crs\modsecurity_iis.conf d:\test.dat + -c d:\temp\antixss.conf -u d:\temp\modsec_urls.txt d:\temp\test1.dat WindowsLocalDebugger $(TargetPath) false