From: Scott Cantor Date: Tue, 30 Oct 2007 18:53:12 +0000 (+0000) Subject: Port up URI sanitizer from branch. X-Git-Tag: 2.0-beta2~54 X-Git-Url: http://www.project-moonshot.org/gitweb/?a=commitdiff_plain;ds=sidebyside;h=7bc8338fb17dceea36edf2d97c055147b1e72327;p=shibboleth%2Fcpp-sp.git Port up URI sanitizer from branch. --- diff --git a/apache/mod_apache.cpp b/apache/mod_apache.cpp index 3610d4f..46250dc 100644 --- a/apache/mod_apache.cpp +++ b/apache/mod_apache.cpp @@ -314,6 +314,8 @@ public: m_dc = (shib_dir_config*)ap_get_module_config(req->per_dir_config, &mod_shib); m_rc = (shib_request_config*)ap_get_module_config(req->request_config, &mod_shib); m_req = req; + + setRequestURI(m_req->unparsed_uri); } virtual ~ShibTargetApache() {} @@ -326,9 +328,6 @@ public: int getPort() const { return ap_get_server_port(m_req); } - const char* getRequestURI() const { - return m_req->unparsed_uri; - } const char* getMethod() const { return m_req->method; } diff --git a/fastcgi/shibauthorizer.cpp b/fastcgi/shibauthorizer.cpp index 284793a..8fdaf3f 100644 --- a/fastcgi/shibauthorizer.cpp +++ b/fastcgi/shibauthorizer.cpp @@ -87,6 +87,8 @@ public: if (!server_scheme_str || !*server_scheme_str) server_scheme_str = (m_port == 443 || m_port == 8443) ? "https" : "http"; m_scheme = server_scheme_str; + + setRequestURI(FCGX_GetParam("REQUEST_URI", m_req->envp)); } ~ShibTargetFCGIAuth() { } @@ -100,9 +102,6 @@ public: int getPort() const { return m_port; } - const char* getRequestURI() const { - return FCGX_GetParam("REQUEST_URI", m_req->envp); - } const char* getMethod() const { return FCGX_GetParam("REQUEST_METHOD", m_req->envp); } diff --git a/fastcgi/shibresponder.cpp b/fastcgi/shibresponder.cpp index ce42928..247eb7c 100644 --- a/fastcgi/shibresponder.cpp +++ b/fastcgi/shibresponder.cpp @@ -89,6 +89,8 @@ public: if (!server_scheme_str || !*server_scheme_str) server_scheme_str = (m_port == 443 || m_port == 8443) ? "https" : "http"; m_scheme = server_scheme_str; + + setRequestURI(FCGX_GetParam("REQUEST_URI", m_req->envp)); } ~ShibTargetFCGI() { } @@ -102,9 +104,6 @@ public: int getPort() const { return m_port; } - const char* getRequestURI() const { - return FCGX_GetParam("REQUEST_URI", m_req->envp); - } const char* getMethod() const { return FCGX_GetParam("REQUEST_METHOD", m_req->envp); } diff --git a/isapi_shib/isapi_shib.cpp b/isapi_shib/isapi_shib.cpp index ae58d5d..25b0a44 100644 --- a/isapi_shib/isapi_shib.cpp +++ b/isapi_shib/isapi_shib.cpp @@ -355,7 +355,7 @@ class ShibTargetIsapiF : public AbstractSPRequest PHTTP_FILTER_PREPROC_HEADERS m_pn; multimap m_headers; int m_port; - string m_scheme,m_hostname,m_uri; + string m_scheme,m_hostname; mutable string m_remote_addr,m_content_type,m_method; dynabuf m_allhttp; @@ -366,7 +366,7 @@ public: // URL path always come from IIS. dynabuf var(256); GetHeader(pn,pfc,"url",var,256,false); - m_uri = var; + setRequestURI(var); // Port may come from IIS or from site def. if (!g_bNormalizeRequest || (pfc->fIsSecurePort && site.m_sslport.empty()) || (!pfc->fIsSecurePort && site.m_port.empty())) { @@ -403,9 +403,6 @@ public: int getPort() const { return m_port; } - const char* getRequestURI() const { - return m_uri.c_str(); - } const char* getMethod() const { if (m_method.empty()) { dynabuf var(5); @@ -697,25 +694,29 @@ public: * PathInfo: /SAML/POST */ + string uri; + // Clearly we're only in bad mode if path info exists at all. if (lpECB->lpszPathInfo && *(lpECB->lpszPathInfo)) { if (strstr(lpECB->lpszPathInfo,url)) // Pretty good chance we're in bad mode, unless the PathInfo repeats the path itself. - m_uri = lpECB->lpszPathInfo; + uri = lpECB->lpszPathInfo; else { - m_uri = url; - m_uri += lpECB->lpszPathInfo; + uri = url; + uri += lpECB->lpszPathInfo; } } else { - m_uri = url; + uri = url; } // For consistency with Apache, let's add the query string. if (lpECB->lpszQueryString && *(lpECB->lpszQueryString)) { - m_uri += '?'; - m_uri += lpECB->lpszQueryString; + uri += '?'; + uri += lpECB->lpszQueryString; } + + setRequestURI(uri.c_str()); } ~ShibTargetIsapiE() { } @@ -728,9 +729,6 @@ public: int getPort() const { return m_port; } - const char* getRequestURI() const { - return m_uri.c_str(); - } const char* getMethod() const { return m_lpECB->lpszMethod; } diff --git a/nsapi_shib/nsapi_shib.cpp b/nsapi_shib/nsapi_shib.cpp index d7ee771..13497db 100644 --- a/nsapi_shib/nsapi_shib.cpp +++ b/nsapi_shib/nsapi_shib.cpp @@ -186,7 +186,6 @@ extern "C" NSAPI_PUBLIC int nsapi_shib_init(pblock* pb, ::Session* sn, Request* class ShibTargetNSAPI : public AbstractSPRequest { - string m_uri; mutable string m_body; mutable bool m_gotBody; mutable vector m_certs; @@ -201,13 +200,15 @@ public: const char* uri=pblock_findval("uri", rq->reqpb); const char* qstr=pblock_findval("query", rq->reqpb); - if (uri) - m_uri = uri; - if (qstr) - m_uri = m_uri + '?' + qstr; - } - ~ShibTargetNSAPI() { + if (qstr) { + string temp = string(uri) + '?' + qstr; + setRequestURI(temp.c_str()); + } + else { + setRequestURI(uri); + } } + ~ShibTargetNSAPI() { } const char* getScheme() const { return security_active ? "https" : "http"; @@ -227,9 +228,6 @@ public: int getPort() const { return server_portnum; } - const char* getRequestURI() const { - return m_uri.c_str(); - } const char* getMethod() const { return pblock_findval("method", m_rq->reqpb); } diff --git a/schemas/shibboleth-2.0-native-sp-config.xsd b/schemas/shibboleth-2.0-native-sp-config.xsd index b79b553..0aeb718 100644 --- a/schemas/shibboleth-2.0-native-sp-config.xsd +++ b/schemas/shibboleth-2.0-native-sp-config.xsd @@ -282,11 +282,11 @@ + - diff --git a/shibsp/AbstractSPRequest.cpp b/shibsp/AbstractSPRequest.cpp index 103c2b3..dffe00c 100644 --- a/shibsp/AbstractSPRequest.cpp +++ b/shibsp/AbstractSPRequest.cpp @@ -113,7 +113,55 @@ Session* AbstractSPRequest::getSession(bool checkTimeout, bool ignoreAddress, bo return session; } -const char* AbstractSPRequest::getRequestURL() const { +static char _x2c(const char *what) +{ + register char digit; + + digit = (what[0] >= 'A' ? ((what[0] & 0xdf) - 'A')+10 : (what[0] - '0')); + digit *= 16; + digit += (what[1] >= 'A' ? ((what[1] & 0xdf) - 'A')+10 : (what[1] - '0')); + return(digit); +} + +void AbstractSPRequest::setRequestURI(const char* uri) +{ + // Fix for bug 574, secadv 20061002 + // Unescape URI up to query string delimiter by looking for %XX escapes. + // Adapted from Apache's util.c, ap_unescape_url function. + if (uri) { + while (*uri) { + if (*uri == '?') { + m_uri += uri; + break; + } + else if (*uri == ';') { + // If this is Java being stupid, skip everything up to the query string, if any. + if (!strncmp(uri, ";jsessionid=", 12)) { + if (uri = strchr(uri, '?')) + m_uri += uri; + break; + } + else { + m_uri += *uri; + } + } + else if (*uri != '%') { + m_uri += *uri; + } + else { + ++uri; + if (!isxdigit(*uri) || !isxdigit(*(uri+1))) + throw ConfigurationException("Bad request, contained unsupported encoded characters."); + m_uri += _x2c(uri); + ++uri; + } + ++uri; + } + } +} + +const char* AbstractSPRequest::getRequestURL() const +{ if (m_url.empty()) { // Compute the full target URL int port = getPort(); @@ -124,9 +172,7 @@ const char* AbstractSPRequest::getRequestURL() const { portstr << port; m_url += ":" + portstr.str(); } - scheme = getRequestURI(); - if (scheme) - m_url += scheme; + m_url += m_uri; } return m_url.c_str(); } diff --git a/shibsp/AbstractSPRequest.h b/shibsp/AbstractSPRequest.h index aed5812..f1b64b1 100644 --- a/shibsp/AbstractSPRequest.h +++ b/shibsp/AbstractSPRequest.h @@ -42,6 +42,14 @@ namespace shibsp { protected: AbstractSPRequest(); + /** + * Stores a normalized request URI to ensure it contains no %-encoded characters + * or other undesirable artifacts, such as ;jsessionid appendage. + * + * @param uri the request URI as obtained from the client + */ + void setRequestURI(const char* uri); + public: virtual ~AbstractSPRequest(); @@ -55,6 +63,10 @@ namespace shibsp { Session* getSession(bool checkTimeout=true, bool ignoreAddress=false, bool cache=true) const; + const char* getRequestURI() const { + return m_uri.c_str(); + } + const char* getRequestURL() const; const char* getParameter(const char* name) const; @@ -76,6 +88,7 @@ namespace shibsp { mutable const Application* m_app; mutable bool m_sessionTried; mutable Session* m_session; + std::string m_uri; mutable std::string m_url; void* m_log; // declared void* to avoid log4cpp header conflicts in Apache mutable std::string m_handlerURL;