Security update for shibboleth-sp in etch
Russ Allbery
rra at debian.org
Fri Dec 4 00:16:33 UTC 2009
Moritz Muehlenhoff <jmm at inutil.org> writes:
> On Tue, Dec 01, 2009 at 06:12:16PM -0800, Russ Allbery wrote:
>> I'm very sorry about how long it's taken me to prepare these patches.
>> This should address CVE 2009-3300 in the shibboleth-sp (not
>> shibboleth-sp2) packages in Debian lenny. I will also work on a
>> backport of these patches to the version that released with Debian
>> etch.
>>
>> Note that the upstream source contains Windows line endings in some
>> places, which my mailer doesn't want to send without encoding, so this
>> patch may require the ignore whitespace flag to apply as-is.
>>
>> Please let me know if these are good for upload to the stable-security
>> queue.
> Looks fine, please upload. I'll take care of the update.
The stable-security update has been uploaded. Here is the corresponding
fix for oldstable. Let me know if I have approval to upload this to the
security queue as well.
diff -u shibboleth-sp-1.3f.dfsg1/debian/changelog shibboleth-sp-1.3f.dfsg1/debian/changelog
--- shibboleth-sp-1.3f.dfsg1/debian/changelog
+++ shibboleth-sp-1.3f.dfsg1/debian/changelog
@@ -1,3 +1,11 @@
+shibboleth-sp (1.3f.dfsg1-2+etch2) oldstable-security; urgency=high
+
+ * SECURITY: Fix improper handling of URLs that could be abused for
+ script injection and other cross-site scripting attacks.
+ (CVE-2009-3300)
+
+ -- Russ Allbery <rra at debian.org> Wed, 02 Dec 2009 22:15:50 -0800
+
shibboleth-sp (1.3f.dfsg1-2+etch1) oldstable-security; urgency=high
* SECURITY: Correctly handle decoding of malformed URLs, closing a
diff -u shibboleth-sp-1.3f.dfsg1/debian/patches/series shibboleth-sp-1.3f.dfsg1/debian/patches/series
--- shibboleth-sp-1.3f.dfsg1/debian/patches/series
+++ shibboleth-sp-1.3f.dfsg1/debian/patches/series
@@ -5,0 +6 @@
+CVE-2009-3300
only in patch2:
unchanged:
--- shibboleth-sp-1.3f.dfsg1.orig/debian/patches/CVE-2009-3300
+++ shibboleth-sp-1.3f.dfsg1/debian/patches/CVE-2009-3300
@@ -0,0 +1,216 @@
+SECURITY: Fix improper handling of URLs that could be abused for
+script injection and other cross-site scripting attacks.
+(CVE-2009-3300)
+
+--- shibboleth-sp.orig/adfs/handlers.cpp
++++ shibboleth-sp/adfs/handlers.cpp
+@@ -429,8 +429,19 @@
+ ret=handler->getString("ResponseLocation").second;
+ if (!ret)
+ ret=st->getApplication()->getString("homeURL").second;
+- if (!ret)
+- ret="/";
++ if (!ret) {
++ // No homeURL, so compute a URL to the root of the site.
++ int port = st->getPort();
++ const char* scheme = st->getProtocol();
++ string dest = string(scheme) + "://" + st->getHostname();
++ if ((!strcmp(scheme,"http") && port!=80) || (!strcmp(scheme,"https") && port!=443)) {
++ ostringstream portstr;
++ portstr << port;
++ dest += ':' + portstr.str();
++ }
++ dest += '/';
++ return make_pair(true, st->sendRedirect(dest));
++ }
+ return make_pair(true, st->sendRedirect(ret));
+ }
+
+@@ -469,7 +480,7 @@
+
+ if (target=="default") {
+ pair<bool,const char*> homeURL=app->getString("homeURL");
+- target=homeURL.first ? homeURL.second : "/";
++ target=homeURL.first ? homeURL.second : "";
+ }
+ else if (target=="cookie" || target.empty()) {
+ // Pull the target value from the "relay state" cookie.
+@@ -478,7 +489,7 @@
+ if (!relay_state || !*relay_state) {
+ // No apparent relay state value to use, so fall back on the default.
+ pair<bool,const char*> homeURL=app->getString("homeURL");
+- target=homeURL.first ? homeURL.second : "/";
++ target=homeURL.first ? homeURL.second : "";
+ }
+ else {
+ char* rscopy=strdup(relay_state);
+@@ -519,6 +530,19 @@
+ }
+ }
+
++ if (target == "") {
++ // No homeURL, so compute a URL to the root of the site.
++ int port = st->getPort();
++ const char* scheme = st->getProtocol();
++ target = string(scheme) + "://" + st->getHostname();
++ if ((!strcmp(scheme,"http") && port!=80) || (!strcmp(scheme,"https") && port!=443)) {
++ ostringstream portstr;
++ portstr << port;
++ target += ':' + portstr.str();
++ }
++ target += '/';
++ }
++
+ // Now redirect to the target.
+ return make_pair(true, st->sendRedirect(target));
+ }
+--- shibboleth-sp.orig/apache/mod_apache.cpp
++++ shibboleth-sp/apache/mod_apache.cpp
+@@ -68,6 +68,7 @@
+ char* g_szSHIBConfig = NULL;
+ char* g_szSchemaDir = NULL;
+ ShibTargetConfig* g_Config = NULL;
++ set<string> g_allowedSchemes;
+ string g_unsetHeaderValue;
+ static const char* g_UserDataKey = "_shib_check_user_";
+ }
+@@ -299,9 +300,12 @@
+ const string& content_type="text/html",
+ const Iterator<header_t>& headers=EMPTY(header_t)
+ ) {
++ checkString(content_type, "Detected control character in a response header.");
+ m_req->content_type = ap_psprintf(m_req->pool, content_type.c_str());
+ while (headers.hasNext()) {
+ const header_t& h=headers.next();
++ checkString(h.first, "Detected control character in a response header.");
++ checkString(h.second, "Detected control character in a response header.");
+ ap_table_set(m_req->headers_out, h.first.c_str(), h.second.c_str());
+ }
+ ap_send_http_header(m_req);
+@@ -309,6 +313,9 @@
+ return (void*)((code==200) ? DONE : code);
+ }
+ virtual void* sendRedirect(const string& url) {
++ checkString(url, "Detected control character in an attempted redirect.");
++ if (g_allowedSchemes.find(url.substr(0, url.find(':'))) == g_allowedSchemes.end())
++ throw FatalProfileException("Invalid scheme in attempted redirect.");
+ ap_table_set(m_req->headers_out, "Location", url.c_str());
+ return (void*)REDIRECT;
+ }
+@@ -318,6 +325,15 @@
+ request_rec* m_req;
+ shib_dir_config* m_dc;
+ shib_server_config* m_sc;
++
++private:
++ void checkString(const string& s, const char* msg) {
++ string::const_iterator e = s.end();
++ for (string::const_iterator i=s.begin(); i!=e; ++i) {
++ if (iscntrl(*i))
++ throw FatalProfileException(msg);
++ }
++ }
+ };
+
+ /********************************************************************************/
+@@ -943,9 +959,25 @@
+ Locker locker(conf);
+ const IPropertySet* props=conf->getPropertySet("Local");
+ if (props) {
+- pair<bool,const char*> unsetValue=props->getString("unsetHeaderValue");
+- if (unsetValue.first)
+- g_unsetHeaderValue = unsetValue.second;
++ pair<bool,const char*> str=props->getString("unsetHeaderValue");
++ if (str.first)
++ g_unsetHeaderValue = str.second;
++ str=props->getString("allowedSchemes");
++ if (str.first) {
++ string schemes=str.second;
++ unsigned int j=0;
++ for (unsigned int i=0; i < schemes.length(); i++) {
++ if (schemes.at(i)==' ') {
++ g_allowedSchemes.insert(schemes.substr(j, i-j));
++ j = i+1;
++ }
++ }
++ g_allowedSchemes.insert(schemes.substr(j, schemes.length()-j));
++ }
++ }
++ if (g_allowedSchemes.empty()) {
++ g_allowedSchemes.insert("https");
++ g_allowedSchemes.insert("http");
+ }
+ }
+ catch (...) {
+--- shibboleth-sp.orig/schemas/shibboleth-targetconfig-1.0.xsd
++++ shibboleth-sp/schemas/shibboleth-targetconfig-1.0.xsd
+@@ -179,6 +179,7 @@
+ <attribute name="logger" type="anyURI" use="optional"/>
+ <attribute name="localRelayState" type="boolean" use="optional" default="false"/>
+ <attribute name="unsetHeaderValue" type="string" use="optional"/>
++ <attribute name="allowedSchemes" type="conf:listOfStrings"/>
+ <anyAttribute namespace="##other" processContents="lax"/>
+ </complexType>
+
+--- shibboleth-sp.orig/shib-target/shib-handlers.cpp
++++ shibboleth-sp/shib-target/shib-handlers.cpp
+@@ -316,7 +316,7 @@
+
+ if (target=="default") {
+ pair<bool,const char*> homeURL=app->getString("homeURL");
+- target=homeURL.first ? homeURL.second : "/";
++ target=homeURL.first ? homeURL.second : "";
+ }
+ else if (target=="cookie" || target.empty()) {
+ // Pull the target value from the "relay state" cookie.
+@@ -325,7 +325,7 @@
+ if (!relay_state || !*relay_state) {
+ // No apparent relay state value to use, so fall back on the default.
+ pair<bool,const char*> homeURL=app->getString("homeURL");
+- target=homeURL.first ? homeURL.second : "/";
++ target=homeURL.first ? homeURL.second : "";
+ }
+ else {
+ char* rscopy=strdup(relay_state);
+@@ -366,6 +366,19 @@
+ }
+ }
+
++ if (target == "") {
++ // No homeURL, so compute a URL to the root of the site.
++ int port = st->getPort();
++ const char* scheme = st->getProtocol();
++ target = string(scheme) + "://" + st->getHostname();
++ if ((!strcmp(scheme,"http") && port!=80) || (!strcmp(scheme,"https") && port!=443)) {
++ ostringstream portstr;
++ portstr << port;
++ target += ':' + portstr.str();
++ }
++ target += '/';
++ }
++
+ // Now redirect to the target.
+ return make_pair(true, st->sendRedirect(target));
+ }
+@@ -401,8 +414,19 @@
+ ret=handler->getString("ResponseLocation").second;
+ if (!ret)
+ ret=st->getApplication()->getString("homeURL").second;
+- if (!ret)
+- ret="/";
++ if (!ret) {
++ // No homeURL, so compute a URL to the root of the site.
++ int port = st->getPort();
++ const char* scheme = st->getProtocol();
++ string dest = string(scheme) + "://" + st->getHostname();
++ if ((!strcmp(scheme,"http") && port!=80) || (!strcmp(scheme,"https") && port!=443)) {
++ ostringstream portstr;
++ portstr << port;
++ dest += ':' + portstr.str();
++ }
++ dest += '/';
++ return make_pair(true, st->sendRedirect(dest));
++ }
+ return make_pair(true, st->sendRedirect(ret));
+ }
+
--
Russ Allbery (rra at debian.org) <http://www.eyrie.org/~eagle/>
More information about the Pkg-shibboleth-devel
mailing list