Author: rjung Date: Wed Jan 2 11:51:44 2008 New Revision: 608199 URL: http://svn.apache.org/viewvc?rev=608199&view=rev Log: Improve XSS hardening of status worker.
Modified: tomcat/connectors/trunk/jk/native/common/jk_status.c tomcat/connectors/trunk/jk/xdocs/miscellaneous/changelog.xml Modified: tomcat/connectors/trunk/jk/native/common/jk_status.c URL: http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/native/common/jk_status.c?rev=608199&r1=608198&r2=608199&view=diff ============================================================================== --- tomcat/connectors/trunk/jk/native/common/jk_status.c (original) +++ tomcat/connectors/trunk/jk/native/common/jk_status.c Wed Jan 2 11:51:44 2008 @@ -148,7 +148,7 @@ #define JK_STATUS_WAIT_AFTER_UPDATE "3" #define JK_STATUS_REFRESH_DEF "10" -#define JK_STATUS_ESC_CHARS ("<>?&") +#define JK_STATUS_ESC_CHARS ("<>?\"") #define JK_STATUS_HEAD "<?xml version=\"1.0\" encoding=\"ISO-8859-1\"?>\n" \ "<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Transitional//EN\"" \ @@ -247,6 +247,7 @@ { status_worker_t *worker; + char *query_string; jk_map_t *req_params; char *msg; @@ -936,13 +937,6 @@ JK_TRACE_ENTER(l); - if (!jk_map_alloc(&(p->req_params))) { - jk_log(l, JK_LOG_ERROR, - "Status worker '%s' could not alloc map for request parameters", - w->name); - JK_TRACE_EXIT(l); - return JK_FALSE; - } if (!s->query_string) { if (JK_IS_DEBUG_LEVEL(l)) jk_log(l, JK_LOG_DEBUG, @@ -951,15 +945,39 @@ JK_TRACE_EXIT(l); return JK_TRUE; } + + p->query_string = jk_pool_strdup(s->pool, s->query_string); + if (!p->query_string) { + jk_log(l, JK_LOG_ERROR, + "Status worker '%s' could not copy query string", + w->name); + JK_TRACE_EXIT(l); + return JK_FALSE; + } + + /* XXX We simply mask special chars n the query string with '@' to prevent cross site scripting */ + query = p->query_string; + while ((query = strpbrk(query, JK_STATUS_ESC_CHARS))) + query[0] = '@'; + + if (!jk_map_alloc(&(p->req_params))) { + jk_log(l, JK_LOG_ERROR, + "Status worker '%s' could not alloc map for request parameters", + w->name); + JK_TRACE_EXIT(l); + return JK_FALSE; + } m = p->req_params; - query = jk_pool_strdup(s->pool, s->query_string); + + query = jk_pool_strdup(s->pool, p->query_string); if (!query) { jk_log(l, JK_LOG_ERROR, - "Status worker '%s' could not copy string", + "Status worker '%s' could not copy query string", w->name); JK_TRACE_EXIT(l); return JK_FALSE; } + #ifdef _REENTRANT for (param = strtok_r(query, "&", &lasts); param; param = strtok_r(NULL, "&", &lasts)) { @@ -977,14 +995,9 @@ } value = strchr(key, '='); if (value) { - char *off; *value = '\0'; value++; /* XXX Depending on the params values, we might need to trim and decode */ - /* XXX For now we simply mask special chars with '@' to prevent cross code injection */ - off = value; - while ((off = strpbrk(off, JK_STATUS_ESC_CHARS))) - off[0] = '@'; if (strlen(key)) { if (JK_IS_DEBUG_LEVEL(l)) jk_log(l, JK_LOG_DEBUG, @@ -3336,7 +3349,7 @@ cmd == JK_STATUS_CMD_SHOW) && refresh > 0) { jk_printf(s, "\n<meta http-equiv=\"Refresh\" content=\"%d;url=%s?%s\">", - refresh, s->req_uri, s->query_string); + refresh, s->req_uri, p->query_string); } if (w->css) { jk_putv(s, "\n<link rel=\"stylesheet\" type=\"text/css\" href=\"", @@ -3362,7 +3375,7 @@ if (cmd == JK_STATUS_CMD_LIST || cmd == JK_STATUS_CMD_SHOW) { if (refresh > 0) { - const char *str = s->query_string; + const char *str = p->query_string; char *buf = jk_pool_alloc(s->pool, sizeof(char *) * (strlen(str)+1)); int result = 0; size_t scan = 0; Modified: tomcat/connectors/trunk/jk/xdocs/miscellaneous/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/xdocs/miscellaneous/changelog.xml?rev=608199&r1=608198&r2=608199&view=diff ============================================================================== --- tomcat/connectors/trunk/jk/xdocs/miscellaneous/changelog.xml (original) +++ tomcat/connectors/trunk/jk/xdocs/miscellaneous/changelog.xml Wed Jan 2 11:51:44 2008 @@ -44,6 +44,9 @@ <subsection name="Native"> <changelog> <update> + Status: Improve XSS hardening. (rjung) + </update> + <update> Move initialization of service members with defaults from web server specific code to our generic jk_init_ws_service() function. (rjung) </update> --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]