Port up URI sanitizer from branch.

git-svn-id: https://svn.middleware.georgetown.edu/cpp-sp/trunk@2583 cb58f699-b61c-0410-a6fe-9272a202ed29

diff --git a/apache/mod_apache.cpp b/apache/mod_apache.cpp
index 3610d4f..46250dc 100644 (file)
--- 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 (file)
--- a/fastcgi/shibauthorizer.cpp
+++ b/fastcgi/shibauthorizer.cpp
@@ -87,6 +87,8 @@ public:
         if (!server_scheme_str || !*server_scheme_str)\r
             server_scheme_str = (m_port == 443 || m_port == 8443) ? "https" : "http";\r
         m_scheme = server_scheme_str;\r
+\r
+        setRequestURI(FCGX_GetParam("REQUEST_URI", m_req->envp));\r
     }\r
 \r
     ~ShibTargetFCGIAuth() { }\r
@@ -100,9 +102,6 @@ public:
     int getPort() const {\r
         return m_port;\r
     }\r
-    const char* getRequestURI() const {\r
-        return FCGX_GetParam("REQUEST_URI", m_req->envp);\r
-    }\r
     const char* getMethod() const {\r
         return FCGX_GetParam("REQUEST_METHOD", m_req->envp);\r
     }\r

diff --git a/fastcgi/shibresponder.cpp b/fastcgi/shibresponder.cpp
index ce42928..247eb7c 100644 (file)
--- a/fastcgi/shibresponder.cpp
+++ b/fastcgi/shibresponder.cpp
@@ -89,6 +89,8 @@ public:
         if (!server_scheme_str || !*server_scheme_str)\r
             server_scheme_str = (m_port == 443 || m_port == 8443) ? "https" : "http";\r
         m_scheme = server_scheme_str;\r
+\r
+        setRequestURI(FCGX_GetParam("REQUEST_URI", m_req->envp));\r
     }\r
 \r
     ~ShibTargetFCGI() { }\r
@@ -102,9 +104,6 @@ public:
     int getPort() const {\r
         return m_port;\r
     }\r
-    const char* getRequestURI() const {\r
-        return FCGX_GetParam("REQUEST_URI", m_req->envp);\r
-    }\r
     const char* getMethod() const {\r
         return FCGX_GetParam("REQUEST_METHOD", m_req->envp);\r
     }\r

diff --git a/isapi_shib/isapi_shib.cpp b/isapi_shib/isapi_shib.cpp
index ae58d5d..25b0a44 100644 (file)
--- 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<string,string> 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 (file)
--- 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<string> 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 (file)
--- a/schemas/shibboleth-2.0-native-sp-config.xsd
+++ b/schemas/shibboleth-2.0-native-sp-config.xsd
@@ -282,11 +282,11 @@
                <attribute name="forceAuthn" type="boolean"/>\r
                <attribute name="authnContextClassRef" type="anyURI"/>\r
                <attribute name="authnContextComparison" type="samlp:AuthnContextComparisonType"/>\r
+        <attribute name="redirectErrors" type="anyURI"/>\r
                <attribute name="sessionError" type="anyURI"/>\r
                <attribute name="metadataError" type="anyURI"/>\r
                <attribute name="accessError" type="anyURI"/>\r
                <attribute name="sslError" type="anyURI"/>\r
-        <attribute name="redirectErrors" type="anyURI"/>\r
                <anyAttribute namespace="##other" processContents="lax"/>\r
        </attributeGroup>\r
        <element name="AccessControlProvider" type="conf:PluggableType"/>\r

diff --git a/shibsp/AbstractSPRequest.cpp b/shibsp/AbstractSPRequest.cpp
index 103c2b3..dffe00c 100644 (file)
--- 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)\r
+{\r
+    register char digit;\r
+\r
+    digit = (what[0] >= 'A' ? ((what[0] & 0xdf) - 'A')+10 : (what[0] - '0'));\r
+    digit *= 16;\r
+    digit += (what[1] >= 'A' ? ((what[1] & 0xdf) - 'A')+10 : (what[1] - '0'));\r
+    return(digit);\r
+}\r
+
+void AbstractSPRequest::setRequestURI(const char* uri)
+{
+    // Fix for bug 574, secadv 20061002\r
+    // Unescape URI up to query string delimiter by looking for %XX escapes.\r
+    // Adapted from Apache's util.c, ap_unescape_url function.\r
+    if (uri) {\r
+        while (*uri) {\r
+            if (*uri == '?') {\r
+                m_uri += uri;\r
+                break;\r
+            }\r
+            else if (*uri == ';') {\r
+                // If this is Java being stupid, skip everything up to the query string, if any.\r
+                if (!strncmp(uri, ";jsessionid=", 12)) {\r
+                    if (uri = strchr(uri, '?'))\r
+                        m_uri += uri;\r
+                    break;\r
+                }\r
+                else {\r
+                    m_uri += *uri;\r
+                }\r
+            }\r
+            else if (*uri != '%') {\r
+                m_uri += *uri;\r
+            }\r
+            else {\r
+                ++uri;\r
+                if (!isxdigit(*uri) || !isxdigit(*(uri+1)))\r
+                    throw ConfigurationException("Bad request, contained unsupported encoded characters.");\r
+                m_uri += _x2c(uri);\r
+                ++uri;\r
+            }\r
+            ++uri;\r
+        }\r
+    }\r
+}
+
+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 (file)
--- 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;