Author: rjung Date: Mon Dec 29 12:30:29 2014 New Revision: 1648318 URL: http://svn.apache.org/r1648318 Log: Enforce implementation restriction on maximal length "60" of worker attributes "name", "host", "route", "domain", "redirect", "session_cookie", "session_path" and "set_session_cookie".
Checks were added to configuration file processing and configuration updates via the status worker. Next: document limits. Modified: tomcat/jk/trunk/native/common/jk_ajp_common.c tomcat/jk/trunk/native/common/jk_lb_worker.c tomcat/jk/trunk/native/common/jk_shm.c tomcat/jk/trunk/native/common/jk_status.c tomcat/jk/trunk/native/common/jk_util.c tomcat/jk/trunk/native/common/jk_util.h tomcat/jk/trunk/native/common/jk_worker.c tomcat/jk/trunk/xdocs/miscellaneous/changelog.xml Modified: tomcat/jk/trunk/native/common/jk_ajp_common.c URL: http://svn.apache.org/viewvc/tomcat/jk/trunk/native/common/jk_ajp_common.c?rev=1648318&r1=1648317&r2=1648318&view=diff ============================================================================== --- tomcat/jk/trunk/native/common/jk_ajp_common.c (original) +++ tomcat/jk/trunk/native/common/jk_ajp_common.c Mon Dec 29 12:30:29 2014 @@ -2795,13 +2795,19 @@ int ajp_validate(jk_worker_t *pThis, } if (pThis && pThis->worker_private) { + const char *tmp; ajp_worker_t *p = pThis->worker_private; p->worker.we = we; p->port = jk_get_worker_port(props, p->name, port); if (!host) { host = "undefined"; } - strncpy(p->host, jk_get_worker_host(props, p->name, host), JK_SHM_STR_SIZ); + tmp = jk_get_worker_host(props, p->name, host); + if (jk_check_attribute_length("host name", tmp, l) == JK_FALSE) { + JK_TRACE_EXIT(l); + return JK_FALSE; + } + strncpy(p->host, tmp, JK_SHM_STR_SIZ); p->prefer_ipv6 = jk_get_worker_prefer_ipv6(props, p->name, JK_FALSE); if (p->s->h.sequence == 0) { /* Initial setup. Modified: tomcat/jk/trunk/native/common/jk_lb_worker.c URL: http://svn.apache.org/viewvc/tomcat/jk/trunk/native/common/jk_lb_worker.c?rev=1648318&r1=1648317&r2=1648318&view=diff ============================================================================== --- tomcat/jk/trunk/native/common/jk_lb_worker.c (original) +++ tomcat/jk/trunk/native/common/jk_lb_worker.c Mon Dec 29 12:30:29 2014 @@ -1693,6 +1693,10 @@ static int JK_METHOD validate(jk_worker_ } memset(p->lb_workers, 0, num_of_workers * sizeof(lb_sub_worker_t)); for (i = 0; i < num_of_workers; i++) { + if (jk_check_attribute_length("host name", worker_names[i], l) == JK_FALSE) { + JK_TRACE_EXIT(l); + return JK_FALSE; + } p->lb_workers[i].s = jk_shm_alloc_lb_sub_worker(&p->p, p->s->h.id, worker_names[i], l); if (p->lb_workers[i].s == NULL) { jk_log(l, JK_LOG_ERROR, @@ -1739,14 +1743,29 @@ static int JK_METHOD validate(jk_worker_ p->max_packet_size = ms; p->lb_workers[i].distance = jk_get_distance(props, worker_names[i]); - if ((s = jk_get_worker_route(props, worker_names[i], NULL))) + if ((s = jk_get_worker_route(props, worker_names[i], NULL))) { + if (jk_check_attribute_length("route", s, l) == JK_FALSE) { + JK_TRACE_EXIT(l); + return JK_FALSE; + } strncpy(p->lb_workers[i].route, s, JK_SHM_STR_SIZ); + } else strncpy(p->lb_workers[i].route, worker_names[i], JK_SHM_STR_SIZ); - if ((s = jk_get_worker_domain(props, worker_names[i], NULL))) + if ((s = jk_get_worker_domain(props, worker_names[i], NULL))) { + if (jk_check_attribute_length("domain", s, l) == JK_FALSE) { + JK_TRACE_EXIT(l); + return JK_FALSE; + } strncpy(p->lb_workers[i].domain, s, JK_SHM_STR_SIZ); - if ((s = jk_get_worker_redirect(props, worker_names[i], NULL))) + } + if ((s = jk_get_worker_redirect(props, worker_names[i], NULL))) { + if (jk_check_attribute_length("redirect", s, l) == JK_FALSE) { + JK_TRACE_EXIT(l); + return JK_FALSE; + } strncpy(p->lb_workers[i].redirect, s, JK_SHM_STR_SIZ); + } p->lb_workers[i].s->lb_value = 0; p->lb_workers[i].s->state = JK_LB_STATE_IDLE; @@ -1836,6 +1855,7 @@ static int JK_METHOD init(jk_worker_t *p jk_worker_env_t *we, jk_logger_t *log) { int i; + const char *s; lb_worker_t *p = (lb_worker_t *)pThis->worker_private; JK_TRACE_ENTER(log); @@ -1862,16 +1882,25 @@ static int JK_METHOD init(jk_worker_t *p p->lbmethod = jk_get_lb_method(props, p->name); p->lblock = jk_get_lb_lock(props, p->name); - strncpy(p->session_cookie, - jk_get_lb_session_cookie(props, p->name, JK_SESSION_IDENTIFIER), - JK_SHM_STR_SIZ); - strncpy(p->session_path, - jk_get_lb_session_path(props, p->name, JK_PATH_SESSION_IDENTIFIER), - JK_SHM_STR_SIZ); + s = jk_get_lb_session_cookie(props, p->name, JK_SESSION_IDENTIFIER); + if (jk_check_attribute_length("session_cookie", s, log) == JK_FALSE) { + JK_TRACE_EXIT(log); + return JK_FALSE; + } + strncpy(p->session_cookie, s, JK_SHM_STR_SIZ); + s = jk_get_lb_session_path(props, p->name, JK_PATH_SESSION_IDENTIFIER); + if (jk_check_attribute_length("session_path", s, log) == JK_FALSE) { + JK_TRACE_EXIT(log); + return JK_FALSE; + } + strncpy(p->session_path, s, JK_SHM_STR_SIZ); p->set_session_cookie = jk_get_lb_set_session_cookie(props, p->name, JK_FALSE); - strncpy(p->session_cookie_path, - jk_get_lb_session_cookie_path(props, p->name, "/"), - JK_SHM_STR_SIZ); + s = jk_get_lb_session_cookie_path(props, p->name, "/"); + if (jk_check_attribute_length("session_cookie_path", s, log) == JK_FALSE) { + JK_TRACE_EXIT(log); + return JK_FALSE; + } + strncpy(p->session_cookie_path, s, JK_SHM_STR_SIZ); JK_INIT_CS(&(p->cs), i); if (i == JK_FALSE) { Modified: tomcat/jk/trunk/native/common/jk_shm.c URL: http://svn.apache.org/viewvc/tomcat/jk/trunk/native/common/jk_shm.c?rev=1648318&r1=1648317&r2=1648318&view=diff ============================================================================== --- tomcat/jk/trunk/native/common/jk_shm.c (original) +++ tomcat/jk/trunk/native/common/jk_shm.c Mon Dec 29 12:30:29 2014 @@ -811,6 +811,10 @@ jk_shm_worker_header_t *jk_shm_alloc_wor unsigned int i; jk_shm_worker_header_t *w = 0; + if (jk_check_attribute_length("name", name, l) == JK_FALSE) { + return NULL; + } + if (jk_shmem.hdr) { jk_shm_lock(); for (i = 0; i < jk_shmem.hdr->h.data.pos; i += JK_SHM_SLOT_SIZE) { Modified: tomcat/jk/trunk/native/common/jk_status.c URL: http://svn.apache.org/viewvc/tomcat/jk/trunk/native/common/jk_status.c?rev=1648318&r1=1648317&r2=1648318&view=diff ============================================================================== --- tomcat/jk/trunk/native/common/jk_status.c (original) +++ tomcat/jk/trunk/native/common/jk_status.c Mon Dec 29 12:30:29 2014 @@ -2818,18 +2818,22 @@ static void form_member(jk_ws_service_t jk_printf(s, l, "value=\"%d\"/></td></tr>\n", wr->lb_factor); jk_putv(s, "<tr><td>", JK_STATUS_ARG_LBM_TEXT_ROUTE, ":</td><td><input name=\"", - JK_STATUS_ARG_LBM_ROUTE, "\" type=\"text\" ", NULL); - jk_printf(s, l, "value=\"%s\"/></td></tr>\n", wr->route); + JK_STATUS_ARG_LBM_ROUTE, "\" type=\"text\" ", + "value=\"", wr->route, NULL); + jk_printf(s, l, "\" maxlength=\"%d\"/></td></tr>\n", + JK_MAX_NAME_LEN); jk_putv(s, "<tr><td>", JK_STATUS_ARG_LBM_TEXT_REDIRECT, ":</td><td><input name=\"", - JK_STATUS_ARG_LBM_REDIRECT, "\" type=\"text\" ", NULL); - jk_putv(s, "value=\"", wr->redirect, NULL); - jk_puts(s, "\"/></td></tr>\n"); + JK_STATUS_ARG_LBM_REDIRECT, "\" type=\"text\" ", + "value=\"", wr->redirect, NULL); + jk_printf(s, l, "\" maxlength=\"%d\"/></td></tr>\n", + JK_MAX_NAME_LEN); jk_putv(s, "<tr><td>", JK_STATUS_ARG_LBM_TEXT_DOMAIN, ":</td><td><input name=\"", - JK_STATUS_ARG_LBM_DOMAIN, "\" type=\"text\" ", NULL); - jk_putv(s, "value=\"", wr->domain, NULL); - jk_puts(s, "\"/></td></tr>\n"); + JK_STATUS_ARG_LBM_DOMAIN, "\" type=\"text\" ", + "value=\"", wr->domain, NULL); + jk_printf(s, l, "\" maxlength=\"%d\"/></td></tr>\n", + JK_MAX_NAME_LEN); jk_putv(s, "<tr><td>", JK_STATUS_ARG_LBM_TEXT_DISTANCE, ":</td><td><input name=\"", JK_STATUS_ARG_LBM_DISTANCE, "\" type=\"text\" ", NULL); @@ -2841,8 +2845,10 @@ static void form_member(jk_ws_service_t jk_puts(s, "<table>\n"); jk_putv(s, "<tr><td>", JK_STATUS_ARG_AJP_TEXT_HOST_STR, ":</td><td><input name=\"", - JK_STATUS_ARG_AJP_HOST_STR, "\" type=\"text\" ", NULL); - jk_printf(s, l, "value=\"%s\"/></td></tr>\n", aw->host); + JK_STATUS_ARG_AJP_HOST_STR, "\" type=\"text\" ", + "value=\"", aw->host, NULL); + jk_printf(s, l, "\" maxlength=\"%d\"/></td></tr>\n", + JK_MAX_NAME_LEN); jk_putv(s, "<tr><td>", JK_STATUS_ARG_AJP_TEXT_PORT, ":</td><td><input name=\"", JK_STATUS_ARG_AJP_PORT, "\" type=\"text\" ", NULL); @@ -3015,15 +3021,18 @@ static void form_all_members(jk_ws_servi } else if (!strcmp(attribute, JK_STATUS_ARG_LBM_ROUTE)) { jk_printf(s, l, "<input name=\"" JK_STATUS_ARG_MULT_VALUE_BASE "%d\" type=\"text\"", i); - jk_putv(s, "value=\"", wr->route, "\"/>\n", NULL); + jk_putv(s, "value=\"", wr->route, NULL); + jk_printf(s, l, "\" maxlength=\"%d\"/>\n", JK_MAX_NAME_LEN); } else if (!strcmp(attribute, JK_STATUS_ARG_LBM_REDIRECT)) { jk_printf(s, l, "<input name=\"" JK_STATUS_ARG_MULT_VALUE_BASE "%d\" type=\"text\"", i); - jk_putv(s, "value=\"", wr->redirect, "\"/>\n", NULL); + jk_putv(s, "value=\"", wr->redirect, NULL); + jk_printf(s, l, "\" maxlength=\"%d\"/>\n", JK_MAX_NAME_LEN); } else if (!strcmp(attribute, JK_STATUS_ARG_LBM_DOMAIN)) { jk_printf(s, l, "<input name=\"" JK_STATUS_ARG_MULT_VALUE_BASE "%d\" type=\"text\"", i); - jk_putv(s, "value=\"", wr->domain, "\"/>\n", NULL); + jk_putv(s, "value=\"", wr->domain, NULL); + jk_printf(s, l, "\" maxlength=\"%d\"/>\n", JK_MAX_NAME_LEN); } else if (!strcmp(attribute, JK_STATUS_ARG_LBM_DISTANCE)) { jk_printf(s, l, "<input name=\"" JK_STATUS_ARG_MULT_VALUE_BASE "%d\" type=\"text\"", i); @@ -3314,7 +3323,15 @@ static int commit_member(jk_ws_service_t *side_effect |= JK_STATUS_NEEDS_UPDATE_MULT | JK_STATUS_NEEDS_PUSH; if ((rv = status_get_string(p, JK_STATUS_ARG_LBM_ROUTE, NULL, &arg, l)) == JK_TRUE) { - if (strncmp(wr->route, arg, JK_SHM_STR_SIZ )) { + if (jk_check_attribute_length("route", arg, l) == JK_FALSE) { + const char *msg = "Update failed (at least partially): new route '%s' " + "too long for sub worker '%s', see log file for details."; + size_t size = strlen(msg) + strlen(arg) + strlen(wr->name) + 1; + p->msg = jk_pool_alloc(s->pool, size); + snprintf(p->msg, size, msg, arg, aw->name); + rc = JK_FALSE; + } + else if (strncmp(wr->route, arg, JK_SHM_STR_SIZ )) { jk_log(l, JK_LOG_INFO, "Status worker '%s' changing 'route' for sub worker '%s' of lb worker '%s' from '%s' to '%s'", w->name, wr->name, lb_name, wr->route, arg); @@ -3332,7 +3349,15 @@ static int commit_member(jk_ws_service_t } if ((rv = status_get_string(p, JK_STATUS_ARG_LBM_REDIRECT, NULL, &arg, l)) == JK_TRUE) { - if (strncmp(wr->redirect, arg, JK_SHM_STR_SIZ )) { + if (jk_check_attribute_length("redirect", arg, l) == JK_FALSE) { + const char *msg = "Update failed (at least partially): new redirect '%s' " + "too long for sub worker '%s', see log file for details."; + size_t size = strlen(msg) + strlen(arg) + strlen(wr->name) + 1; + p->msg = jk_pool_alloc(s->pool, size); + snprintf(p->msg, size, msg, arg, aw->name); + rc = JK_FALSE; + } + else if (strncmp(wr->redirect, arg, JK_SHM_STR_SIZ )) { jk_log(l, JK_LOG_INFO, "Status worker '%s' changing 'redirect' for sub worker '%s' of lb worker '%s' from '%s' to '%s'", w->name, wr->name, lb_name, wr->redirect, arg); @@ -3342,7 +3367,15 @@ static int commit_member(jk_ws_service_t } if ((rv = status_get_string(p, JK_STATUS_ARG_LBM_DOMAIN, NULL, &arg, l)) == JK_TRUE) { - if (strncmp(wr->domain, arg, JK_SHM_STR_SIZ )) { + if (jk_check_attribute_length("domain", arg, l) == JK_FALSE) { + const char *msg = "Update failed (at least partially): new domain '%s' " + "too long for sub worker '%s', see log file for details."; + size_t size = strlen(msg) + strlen(arg) + strlen(wr->name) + 1; + p->msg = jk_pool_alloc(s->pool, size); + snprintf(p->msg, size, msg, arg, aw->name); + rc = JK_FALSE; + } + else if (strncmp(wr->domain, arg, JK_SHM_STR_SIZ )) { jk_log(l, JK_LOG_INFO, "Status worker '%s' changing 'domain' for sub worker '%s' of lb worker '%s' from '%s' to '%s'", w->name, wr->name, lb_name, wr->domain, arg); @@ -3375,7 +3408,15 @@ static int commit_member(jk_ws_service_t } if ((rv = status_get_string(p, JK_STATUS_ARG_AJP_HOST_STR, NULL, &arg, l)) == JK_TRUE) { - if (strncmp(aw->host, arg, JK_SHM_STR_SIZ)) { + if (jk_check_attribute_length("host name", arg, l) == JK_FALSE) { + const char *msg = "Update failed (at least partially): new host name '%s' " + "too long for sub worker '%s', see log file for details."; + size_t size = strlen(msg) + strlen(arg) + strlen(aw->name) + 1; + p->msg = jk_pool_alloc(s->pool, size); + snprintf(p->msg, size, msg, arg, aw->name); + rc = JK_FALSE; + } + else if (strncmp(aw->host, arg, JK_SHM_STR_SIZ)) { jk_log(l, JK_LOG_INFO, "Status worker '%s' changing 'host' for sub worker '%s' from '%s' to '%s'", w->name, aw->name, aw->host, arg); @@ -3622,7 +3663,15 @@ static void commit_all_members(jk_ws_ser } else if (!strcmp(attribute, JK_STATUS_ARG_LBM_ROUTE)) { if (rv == JK_TRUE) { - if (strncmp(wr->route, arg, JK_SHM_STR_SIZ)) { + if (jk_check_attribute_length("route", arg, l) == JK_FALSE) { + const char *msg = "Update failed (at least partially): new route '%s' " + "too long for sub worker '%s', see log file for details."; + size_t size = strlen(msg) + strlen(arg) + strlen(wr->name) + 1; + p->msg = jk_pool_alloc(s->pool, size); + snprintf(p->msg, size, msg, arg, aw->name); + rc = JK_FALSE; + } + else if (strncmp(wr->route, arg, JK_SHM_STR_SIZ)) { jk_log(l, JK_LOG_INFO, "Status worker '%s' changing 'route' for sub worker '%s' of lb worker '%s' from '%s' to '%s'", w->name, wr->name, name, wr->route, arg); @@ -3641,7 +3690,15 @@ static void commit_all_members(jk_ws_ser } else if (!strcmp(attribute, JK_STATUS_ARG_LBM_REDIRECT)) { if (rv == JK_TRUE) { - if (strncmp(wr->redirect, arg, JK_SHM_STR_SIZ)) { + if (jk_check_attribute_length("redirect", arg, l) == JK_FALSE) { + const char *msg = "Update failed (at least partially): new redirect '%s' " + "too long for sub worker '%s', see log file for details."; + size_t size = strlen(msg) + strlen(arg) + strlen(wr->name) + 1; + p->msg = jk_pool_alloc(s->pool, size); + snprintf(p->msg, size, msg, arg, aw->name); + rc = JK_FALSE; + } + else if (strncmp(wr->redirect, arg, JK_SHM_STR_SIZ)) { jk_log(l, JK_LOG_INFO, "Status worker '%s' changing 'redirect' for sub worker '%s' of lb worker '%s' from '%s' to '%s'", w->name, wr->name, name, wr->redirect, arg); @@ -3652,7 +3709,15 @@ static void commit_all_members(jk_ws_ser } else if (!strcmp(attribute, JK_STATUS_ARG_LBM_DOMAIN)) { if (rv == JK_TRUE) { - if (strncmp(wr->domain, arg, JK_SHM_STR_SIZ)) { + if (jk_check_attribute_length("domain", arg, l) == JK_FALSE) { + const char *msg = "Update failed (at least partially): new domain '%s' " + "too long for sub worker '%s', see log file for details."; + size_t size = strlen(msg) + strlen(arg) + strlen(wr->name) + 1; + p->msg = jk_pool_alloc(s->pool, size); + snprintf(p->msg, size, msg, arg, aw->name); + rc = JK_FALSE; + } + else if (strncmp(wr->domain, arg, JK_SHM_STR_SIZ)) { jk_log(l, JK_LOG_INFO, "Status worker '%s' changing 'domain' for sub worker '%s' of lb worker '%s' from '%s' to '%s'", w->name, wr->name, name, wr->domain, arg); Modified: tomcat/jk/trunk/native/common/jk_util.c URL: http://svn.apache.org/viewvc/tomcat/jk/trunk/native/common/jk_util.c?rev=1648318&r1=1648317&r2=1648318&view=diff ============================================================================== --- tomcat/jk/trunk/native/common/jk_util.c (original) +++ tomcat/jk/trunk/native/common/jk_util.c Mon Dec 29 12:30:29 2014 @@ -816,6 +816,20 @@ int jk_log(jk_logger_t *l, return rc; } +int jk_check_attribute_length(const char *name, const char *value, + jk_logger_t *l) +{ + size_t len = strlen(value); + if (len > JK_MAX_NAME_LEN) { + jk_log(l, JK_LOG_ERROR, + "Worker %s '%s' is %d bytes too long, " + "a maximum of %d bytes is supported", + name, value, len - JK_MAX_NAME_LEN, JK_MAX_NAME_LEN); + return JK_FALSE; + } + return JK_TRUE; +} + const char *jk_get_worker_type(jk_map_t *m, const char *wname) { char buf[PARAM_BUFFER_SIZE]; Modified: tomcat/jk/trunk/native/common/jk_util.h URL: http://svn.apache.org/viewvc/tomcat/jk/trunk/native/common/jk_util.h?rev=1648318&r1=1648317&r2=1648318&view=diff ============================================================================== --- tomcat/jk/trunk/native/common/jk_util.h (original) +++ tomcat/jk/trunk/native/common/jk_util.h Mon Dec 29 12:30:29 2014 @@ -58,6 +58,9 @@ int jk_log(jk_logger_t *l, const char *file, int line, const char *funcname, int level, const char *fmt, ...); +int jk_check_attribute_length(const char *name, const char *value, + jk_logger_t *l); + const char *jk_get_worker_host(jk_map_t *m, const char *wname, const char *def); const char *jk_get_worker_type(jk_map_t *m, const char *wname); Modified: tomcat/jk/trunk/native/common/jk_worker.c URL: http://svn.apache.org/viewvc/tomcat/jk/trunk/native/common/jk_worker.c?rev=1648318&r1=1648317&r2=1648318&view=diff ============================================================================== --- tomcat/jk/trunk/native/common/jk_worker.c (original) +++ tomcat/jk/trunk/native/common/jk_worker.c Mon Dec 29 12:30:29 2014 @@ -124,6 +124,10 @@ int wc_create_worker(const char *name, i { JK_TRACE_ENTER(l); + if (jk_check_attribute_length("name", name, l) == JK_FALSE) { + JK_TRACE_EXIT(l); + return JK_FALSE; + } if (rc) { const char *type = jk_get_worker_type(init_data, name); worker_factory fac = get_factory_for(type); Modified: tomcat/jk/trunk/xdocs/miscellaneous/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/jk/trunk/xdocs/miscellaneous/changelog.xml?rev=1648318&r1=1648317&r2=1648318&view=diff ============================================================================== --- tomcat/jk/trunk/xdocs/miscellaneous/changelog.xml (original) +++ tomcat/jk/trunk/xdocs/miscellaneous/changelog.xml Mon Dec 29 12:30:29 2014 @@ -130,6 +130,13 @@ Replace fixed allocation of 32 entries for fail_on_status by dynamic allocation. (rjung) </update> + <add> + Enforce implementation restriction on maximal length "60" of worker + attributes "name", "host", "route", "domain", "redirect", + "session_cookie", "session_path" and "set_session_cookie". Checks were + added to configuration file processing and configuration + updates via the status worker. (rjung) + </add> </changelog> </subsection> </section> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org