[PATCH] Backport fix of CVE-2009-3300 for SP

Ferenc Wagner wferi at niif.hu
Tue Nov 24 15:00:48 UTC 2009


Use internal functions instead of bumping the soname of the XMLTooling
and OpenSAML libraries as the original upstream fix did.  This also entails
hardwiring the newly introduced allowedSchemes configuration option to HTTP and
HTTPS only.  Unfortunately, the new symbols are not fully hidden, because various
components of this package use them from libshibsp.
---
 apache/mod_apache.cpp                   |    3 ++
 cve2009-3300.h                          |    9 ++++++
 debian/changelog                        |    6 ++++
 fastcgi/shibauthorizer.cpp              |    3 ++
 fastcgi/shibresponder.cpp               |    3 ++
 isapi_shib/isapi_shib.cpp               |    6 +++-
 nsapi_shib/nsapi_shib.cpp               |    3 ++
 shibsp/SPConfig.cpp                     |   42 +++++++++++++++++++++++++++++++
 shibsp/handler/impl/AbstractHandler.cpp |   19 ++++++++++---
 shibsp/handler/impl/LogoutHandler.cpp   |   17 ++++++++++--
 shibsp/internal.h                       |    2 +
 11 files changed, 104 insertions(+), 9 deletions(-)
 create mode 100644 cve2009-3300.h

diff --git a/apache/mod_apache.cpp b/apache/mod_apache.cpp
index c5106dd..972d8ac 100644
--- a/apache/mod_apache.cpp
+++ b/apache/mod_apache.cpp
@@ -31,6 +31,7 @@
 # define _CRT_SECURE_NO_DEPRECATE 1
 #endif
 
+#include "cve2009-3300.h"
 #include <shibsp/AbstractSPRequest.h>
 #include <shibsp/AccessControl.h>
 #include <shibsp/exceptions.h>
@@ -490,6 +491,7 @@ public:
       m_req->content_type = ap_psprintf(m_req->pool, type);
   }
   void setResponseHeader(const char* name, const char* value) {
+   HTTPResponse_setResponseHeader(name, value);
 #ifdef SHIB_DEFERRED_HEADERS
    if (!m_rc)
       // this happens on subrequests
@@ -516,6 +518,7 @@ public:
     return DONE;
   }
   long sendRedirect(const char* url) {
+    HTTPResponse_sendRedirect(url);
     ap_table_set(m_req->headers_out, "Location", url);
     return REDIRECT;
   }
diff --git a/cve2009-3300.h b/cve2009-3300.h
new file mode 100644
index 0000000..cb88cf1
--- /dev/null
+++ b/cve2009-3300.h
@@ -0,0 +1,9 @@
+#ifndef CVE2009_3300
+#define CVE2009_3300
+
+namespace shibsp {
+    void HTTPResponse_setResponseHeader(const char* name, const char* value);
+    long HTTPResponse_sendRedirect(const char* url);
+}
+
+#endif
diff --git a/debian/changelog b/debian/changelog
index ff3111e..350614b 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+shibboleth-sp2 (2.0.dfsg1-4+lenny2) stable-security; urgency=high
+
+  * SECURITY: Backport fix for CVE-2009-3300
+
+ -- Ferenc Wagner <wferi at niif.hu>  Tue, 24 Nov 2009 16:02:12 +0100
+
 shibboleth-sp2 (2.0.dfsg1-4+lenny1) stable-security; urgency=high
 
   * Non-maintainer upload.
diff --git a/fastcgi/shibauthorizer.cpp b/fastcgi/shibauthorizer.cpp
index 65fc832..7275353 100644
--- a/fastcgi/shibauthorizer.cpp
+++ b/fastcgi/shibauthorizer.cpp
@@ -26,6 +26,7 @@
 #define _CRT_SECURE_NO_DEPRECATE 1
 #define _SCL_SECURE_NO_WARNINGS 1
 
+#include "cve2009-3300.h"
 #include <shibsp/AbstractSPRequest.h>
 #include <shibsp/SPConfig.h>
 #include <shibsp/ServiceProvider.h>
@@ -157,6 +158,7 @@ public:
         return "";
     }
     void setResponseHeader(const char* name, const char* value) {
+        HTTPResponse_setResponseHeader(name, value);
         // Set for later.
         if (value)
             m_response_headers.insert(make_pair(name,value));
@@ -193,6 +195,7 @@ public:
     }
 
     long sendRedirect(const char* url) {
+        HTTPResponse_sendRedirect(url);
         string hdr=string("Status: 302 Please Wait\r\nLocation: ") + url + "\r\n"
           "Content-Type: text/html\r\n"
           "Content-Length: 40\r\n"
diff --git a/fastcgi/shibresponder.cpp b/fastcgi/shibresponder.cpp
index f40a2e7..a2b1a82 100644
--- a/fastcgi/shibresponder.cpp
+++ b/fastcgi/shibresponder.cpp
@@ -26,6 +26,7 @@
 #define _CRT_SECURE_NO_DEPRECATE 1
 #define _SCL_SECURE_NO_WARNINGS 1
 
+#include "cve2009-3300.h"
 #include <shibsp/AbstractSPRequest.h>
 #include <shibsp/SPConfig.h>
 #include <shibsp/ServiceProvider.h>
@@ -142,6 +143,7 @@ public:
     }
 
     void setResponseHeader(const char* name, const char* value) {
+        HTTPResponse_setResponseHeader(name, value);
         // Set for later.
         if (value)
             m_headers.insert(make_pair(name,value));
@@ -179,6 +181,7 @@ public:
     }
 
     long sendRedirect(const char* url) {
+	HTTPResponse_sendRedirect(url);
         string hdr=string("Status: 302 Please Wait\r\nLocation: ") + url + "\r\n"
           "Content-Type: text/html\r\n"
           "Content-Length: 40\r\n"
diff --git a/isapi_shib/isapi_shib.cpp b/isapi_shib/isapi_shib.cpp
index 1f01f44..54babc2 100644
--- a/isapi_shib/isapi_shib.cpp
+++ b/isapi_shib/isapi_shib.cpp
@@ -26,6 +26,7 @@
 #define _CRT_NONSTDC_NO_DEPRECATE 1
 #define _CRT_SECURE_NO_DEPRECATE 1
 
+#include "cve2009-3300.h"
 #include <shibsp/AbstractSPRequest.h>
 #include <shibsp/SPConfig.h>
 #include <shibsp/ServiceProvider.h>
@@ -484,6 +485,7 @@ public:
     return getHeader("remote-user");
   }
   void setResponseHeader(const char* name, const char* value) {
+    HTTPResponse_setResponseHeader(name, value);
     // Set for later.
     if (value)
         m_headers.insert(make_pair(name,value));
@@ -512,7 +514,7 @@ public:
     return SF_STATUS_REQ_FINISHED;
   }
   long sendRedirect(const char* url) {
-    // XXX: Don't support the httpRedirect option, yet.
+    HTTPResponse_sendRedirect(url);
     string hdr=string("Location: ") + url + "\r\n"
       "Content-Type: text/html\r\n"
       "Content-Length: 40\r\n"
@@ -793,6 +795,7 @@ public:
     return buf.empty() ? "" : buf;
   }
   void setResponseHeader(const char* name, const char* value) {
+    HTTPResponse_setResponseHeader(name, value);
     // Set for later.
     if (value)
         m_headers.insert(make_pair(name,value));
@@ -848,6 +851,7 @@ public:
     return HSE_STATUS_SUCCESS;
   }
   long sendRedirect(const char* url) {
+    HTTPResponse_sendRedirect(url);
     string hdr=string("Location: ") + url + "\r\n"
       "Content-Type: text/html\r\n"
       "Content-Length: 40\r\n"
diff --git a/nsapi_shib/nsapi_shib.cpp b/nsapi_shib/nsapi_shib.cpp
index d1efdc4..2018097 100644
--- a/nsapi_shib/nsapi_shib.cpp
+++ b/nsapi_shib/nsapi_shib.cpp
@@ -33,6 +33,7 @@
 # define _CRT_SECURE_NO_DEPRECATE 1
 #endif
 
+#include "cve2009-3300.h"
 #include <shibsp/AbstractSPRequest.h>
 #include <shibsp/RequestMapper.h>
 #include <shibsp/SPConfig.h>
@@ -340,6 +341,7 @@ public:
     return ru ? ru : "";
   }
   void setResponseHeader(const char* name, const char* value) {
+    HTTPResponse_setResponseHeader(name, value);
     pblock_nvinsert(name, value, m_rq->srvhdrs);
   }
 
@@ -358,6 +360,7 @@ public:
     return REQ_EXIT;
   }
   long sendRedirect(const char* url) {
+    HTTPResponse_sendRedirect(url);
     param_free(pblock_remove("content-type", m_rq->srvhdrs));
     pblock_nninsert("content-length", 0, m_rq->srvhdrs);
     pblock_nvinsert("expires", "01-Jan-1997 12:00:00 GMT", m_rq->srvhdrs);
diff --git a/shibsp/SPConfig.cpp b/shibsp/SPConfig.cpp
index 65d5b74..e17d631 100644
--- a/shibsp/SPConfig.cpp
+++ b/shibsp/SPConfig.cpp
@@ -251,3 +251,45 @@ void SPConfig::term()
 #endif
     log.info("%s library shutdown complete", PACKAGE_STRING);
 }
+
+// Instead of http://svn.middleware.georgetown.edu/view/cpp-xmltooling?view=rev&revision=676
+static void HTTPResponse_sanitizeURL(const char* url)
+{
+    const char* ch;
+    const char* allowedSchemes[] = { "http", "https", NULL };
+
+    for (ch=url; *ch; ++ch) {
+        if (iscntrl(*ch))
+            throw IOException("URL contained a control character.");
+    }
+
+    ch = strchr(url, ':');
+    if (!ch)
+        throw IOException("URL is malformed.");
+    std::string s(url, ch - url);
+
+    for (const char **sch = allowedSchemes; *sch; sch++)
+        if (!strcasecmp(s.c_str(), *sch))
+            return;
+
+    throw IOException("URL contains invalid scheme ($1).", params(1, s.c_str()));
+}
+
+void shibsp::HTTPResponse_setResponseHeader(const char* name, const char* value)
+{
+    for (const char* ch=name; *ch; ++ch) {
+        if (iscntrl(*ch))
+            throw IOException("Response header name contained a control character.");
+    }
+
+    for (const char* ch=value; *ch; ++ch) {
+        if (iscntrl(*ch))
+            throw IOException("Value for response header ($1) contained a control character.", params(1,name));
+    }
+}
+
+long shibsp::HTTPResponse_sendRedirect(const char* url)
+{
+    HTTPResponse_sanitizeURL(url);
+    return HTTPResponse::XMLTOOLING_HTTP_STATUS_MOVED;
+}
diff --git a/shibsp/handler/impl/AbstractHandler.cpp b/shibsp/handler/impl/AbstractHandler.cpp
index 3f2e0da..c274362 100644
--- a/shibsp/handler/impl/AbstractHandler.cpp
+++ b/shibsp/handler/impl/AbstractHandler.cpp
@@ -380,10 +380,19 @@ void AbstractHandler::recoverRelayState(
     // Check for "default" value.
     if (relayState.empty() || relayState == "default") {
         pair<bool,const char*> homeURL=application.getString("homeURL");
-        relayState=homeURL.first ? homeURL.second : "/";
-        return;
+        if (homeURL.first)
+            relayState=homeURL.second;
+        else {
+            // Compute a URL to the root of the site.
+            int port = request.getPort();
+            const char* scheme = request.getScheme();
+            relayState = string(scheme) + "://" + request.getHostname();
+            if ((!strcmp(scheme,"http") && port!=80) || (!strcmp(scheme,"https") && port!=443)) {
+                ostringstream portstr;
+                portstr << port;
+                relayState += ":" + portstr.str();
+            }
+            relayState += '/';
+        }
     }
-
-    if (relayState == "default")
-        relayState.empty();
 }
diff --git a/shibsp/handler/impl/LogoutHandler.cpp b/shibsp/handler/impl/LogoutHandler.cpp
index 3ef1cd2..f8e9bc3 100644
--- a/shibsp/handler/impl/LogoutHandler.cpp
+++ b/shibsp/handler/impl/LogoutHandler.cpp
@@ -61,9 +61,20 @@ pair<bool,long> LogoutHandler::sendLogoutPage(
         return make_pair(true,response.sendResponse(str));
     }
     prop = application.getString("homeURL");
-    if (!prop.first)
-        prop.second = "/";
-    return make_pair(true,response.sendRedirect(prop.second));
+    if (prop.first)
+        return make_pair(true,response.sendRedirect(prop.second));
+
+    // No homeURL, so compute a URL to the root of the site.
+    int port = request.getPort();
+    const char* scheme = request.getScheme();
+    string dest = string(scheme) + "://" + request.getHostname();
+    if ((!strcmp(scheme,"http") && port!=80) || (!strcmp(scheme,"https") && port!=443)) {
+        ostringstream portstr;
+        portstr << port;
+        dest += ":" + portstr.str();
+    }
+    dest += '/';
+    return make_pair(true,response.sendRedirect(dest.c_str()));
 }
 
 pair<bool,long> LogoutHandler::run(SPRequest& request, bool isHandler) const
diff --git a/shibsp/internal.h b/shibsp/internal.h
index 1b5e9e5..7143715 100644
--- a/shibsp/internal.h
+++ b/shibsp/internal.h
@@ -48,4 +48,6 @@
 using namespace xmltooling::logging;
 using namespace xercesc;
 
+#include "cve2009-3300.h"
+
 #endif /* __shibsp_internal_h__ */
-- 
1.5.6.5


--=-=-=--



More information about the Pkg-shibboleth-devel mailing list