[cpp-sp] branch main updated: SSPCPP-906 - SP path sanitization is breaking redirects

[Scott Cantor]

This is an automated email from the git hooks/post-receive script.

scantor pushed a commit to branch main
in repository cpp-sp.

View the commit online:
http://git.shibboleth.net/view/?p=cpp-sp.git;a=commit;h=5ad2f54cca301ea5b8386201ae86d9af1b28d5a5

The following commit(s) were added to refs/heads/main by this push:
       new  5ad2f54c  SSPCPP-906 - SP path sanitization is breaking redirects
5ad2f54c is described below

commit 5ad2f54cca301ea5b8386201ae86d9af1b28d5a5
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Thu Sep 24 14:44:26 2020 -0400

    SSPCPP-906 - SP path sanitization is breaking redirects
    
    https://issues.shibboleth.net/jira/browse/SSPCPP-906
---
 shibsp/AbstractSPRequest.cpp     | 36 ++++--------------------------------
 shibsp/impl/XMLRequestMapper.cpp | 37 +++++++++++++++++++++++++++++++++++--
 2 files changed, 39 insertions(+), 34 deletions(-)

diff --git a/shibsp/AbstractSPRequest.cpp b/shibsp/AbstractSPRequest.cpp
index 5bc3d677..36db1d4b 100644
--- a/shibsp/AbstractSPRequest.cpp
+++ b/shibsp/AbstractSPRequest.cpp
@@ -147,40 +147,12 @@ Session* AbstractSPRequest::getSession(bool checkTimeout, bool ignoreAddress, bo
     return session;
 }
 
-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 != '%') {
-                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;
-        }
-    }
+    if (uri)
+        m_uri = uri;
+    else
+        m_uri.clear();
 }
 
 const char* AbstractSPRequest::getRequestURI() const
diff --git a/shibsp/impl/XMLRequestMapper.cpp b/shibsp/impl/XMLRequestMapper.cpp
index 08e95fba..e7b78c79 100644
--- a/shibsp/impl/XMLRequestMapper.cpp
+++ b/shibsp/impl/XMLRequestMapper.cpp
@@ -364,6 +364,16 @@ Override::Override(bool unicodeAware, const DOMElement* e, Category& log, 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);
+}
+
 const Override* Override::locate(const HTTPRequest& request) const
 {
     // This function is confusing because it's *not* recursive.
@@ -371,11 +381,34 @@ const Override* Override::locate(const HTTPRequest& request) const
     // path parameter starts with the entire request path and
     // we can skip the leading slash as irrelevant.
     const char* path = request.getRequestURI();
-    if (*path == '/')
+    if (path && *path == '/')
         path++;
 
+    // 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.
+    string dup;
+    if (path) {
+        while (*path) {
+            if (*path == '?') {
+                dup += path;
+                break;
+            }
+            else if (*path != '%') {
+                dup += *path;
+            }
+            else {
+                ++path;
+                if (!isxdigit(*path) || !isxdigit(*(path+1)))
+                    throw ConfigurationException("Bad request URI, contained unsupported encoded characters.");
+                dup += _x2c(path);
+                ++path;
+            }
+            ++path;
+        }
+    }
+
     // Now we copy the path, chop the query string, and possibly lower case it.
-    string dup(path);
     string::size_type sep = dup.find('?');
     if (sep != string::npos)
         dup = dup.substr(0, sep);

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.