Bug#985405: src:shibboleth-sp: Error templates allow query-based override of variables
wferi at niif.hu
wferi at niif.hu
Wed Mar 17 17:19:07 GMT 2021
Dear Security Team,
Please review the debdiff below for a buster-security upload.
The advisory text changed a little meanwhile, adding credits to Toni
Huttunen, Fraktal Oy for discovering the problem, mentioning the style
sheet spoofing and adding some mitigation tips.
I haven't got a concrete exploit on hand, but I'll start testing the
updated package shortly.
diff -Nru shibboleth-sp-3.0.4+dfsg1/debian/changelog shibboleth-sp-3.0.4+dfsg1/debian/changelog
--- shibboleth-sp-3.0.4+dfsg1/debian/changelog 2019-03-16 20:51:16.000000000 +0100
+++ shibboleth-sp-3.0.4+dfsg1/debian/changelog 2021-03-17 16:55:49.000000000 +0100
@@ -1,3 +1,26 @@
+shibboleth-sp (3.0.4+dfsg1-1+deb10u1) buster-security; urgency=high
+
+ * [594074b] New patch: SSPCPP-922 - Add externalParameters option to Errors
+ element.
+ Fix a phishing vulnerability: Template generation allows external
+ parameters to override placeholders
+ The primitive template engine used to render error pages allows
+ replacement via query parameters also, though this is not a typical
+ need. Because of this feature, it's possible to cause the SP to
+ display some templates containing values supplied externally by URL
+ manipulation. Though the values are encoded to prevent script
+ injection, the content nevertheless appears to come from the server
+ and so would be interpreted as trustworthy, allowing email addresses,
+ logos, or support URLs to be manipulated by an attacker.
+ This update adds a new <Errors> setting to the configuration called
+ externalParameters, which defaults to false. When false, support for
+ this "feature" is disabled.
+ https://shibboleth.net/community/advisories/secadv_20210317.txt
+ https://issues.shibboleth.net/jira/browse/SSPCPP-922
+ Thanks to Scott Cantor (Closes: #985405)
+
+ -- Ferenc Wágner <wferi at debian.org> Wed, 17 Mar 2021 16:55:49 +0100
+
shibboleth-sp (3.0.4+dfsg1-1) unstable; urgency=medium
* [f284741] New upstream release: 3.0.4
diff -Nru shibboleth-sp-3.0.4+dfsg1/debian/gbp.conf shibboleth-sp-3.0.4+dfsg1/debian/gbp.conf
--- shibboleth-sp-3.0.4+dfsg1/debian/gbp.conf 2019-03-13 20:06:54.000000000 +0100
+++ shibboleth-sp-3.0.4+dfsg1/debian/gbp.conf 2021-03-17 16:21:54.000000000 +0100
@@ -1,5 +1,5 @@
[DEFAULT]
-debian-branch = debian/master
+debian-branch = debian/buster
upstream-branch = upstream/latest
pristine-tar = True
diff -Nru shibboleth-sp-3.0.4+dfsg1/debian/patches/series shibboleth-sp-3.0.4+dfsg1/debian/patches/series
--- shibboleth-sp-3.0.4+dfsg1/debian/patches/series 2019-03-16 20:48:54.000000000 +0100
+++ shibboleth-sp-3.0.4+dfsg1/debian/patches/series 2021-03-17 16:54:46.000000000 +0100
@@ -3,3 +3,4 @@
Debianize-the-systemd-service-file-of-shibd.patch
seckeygen-defaults-for-Debian.patch
Use-runstatedir-from-future-Autoconf-2.70.patch
+SSPCPP-922-Add-externalParameters-option-to-Errors-elemen.patch
diff -Nru shibboleth-sp-3.0.4+dfsg1/debian/patches/SSPCPP-922-Add-externalParameters-option-to-Errors-elemen.patch shibboleth-sp-3.0.4+dfsg1/debian/patches/SSPCPP-922-Add-externalParameters-option-to-Errors-elemen.patch
--- shibboleth-sp-3.0.4+dfsg1/debian/patches/SSPCPP-922-Add-externalParameters-option-to-Errors-elemen.patch 1970-01-01 01:00:00.000000000 +0100
+++ shibboleth-sp-3.0.4+dfsg1/debian/patches/SSPCPP-922-Add-externalParameters-option-to-Errors-elemen.patch 2021-03-17 16:54:46.000000000 +0100
@@ -0,0 +1,125 @@
+From: Scott Cantor <cantor.2 at osu.edu>
+Date: Tue, 16 Mar 2021 10:56:22 -0400
+Subject: SSPCPP-922 - Add externalParameters option to Errors element
+
+https://issues.shibboleth.net/jira/browse/SSPCPP-922
+
+Closes: #985405
+---
+ schemas/shibboleth-3.0-native-sp-config.xsd | 1 +
+ shibsp/ServiceProvider.cpp | 11 +++++++++--
+ shibsp/handler/impl/AttributeCheckerHandler.cpp | 12 ++++++++++--
+ shibsp/handler/impl/FormSessionInitiator.cpp | 12 +++++++++++-
+ shibsp/handler/impl/LogoutHandler.cpp | 10 +++++++++-
+ 5 files changed, 40 insertions(+), 6 deletions(-)
+
+diff --git a/schemas/shibboleth-3.0-native-sp-config.xsd b/schemas/shibboleth-3.0-native-sp-config.xsd
+index 5bfa3a2..3b1a664 100644
+--- a/schemas/shibboleth-3.0-native-sp-config.xsd
++++ b/schemas/shibboleth-3.0-native-sp-config.xsd
+@@ -732,6 +732,7 @@
+ <attribute name="localLogout" type="anyURI"/>
+ <attribute name="globalLogout" type="anyURI"/>
+ <attribute name="partialLogout" type="anyURI"/>
++ <attribute name="externalParameters" type="boolean" />
+ <anyAttribute namespace="##any" processContents="lax"/>
+ </complexType>
+
+diff --git a/shibsp/ServiceProvider.cpp b/shibsp/ServiceProvider.cpp
+index aacbba1..b9f6e4c 100644
+--- a/shibsp/ServiceProvider.cpp
++++ b/shibsp/ServiceProvider.cpp
+@@ -71,9 +71,16 @@ namespace shibsp {
+ if (!app)
+ app = request.getServiceProvider().getApplication(nullptr);
+
+- const PropertySet* props=app->getPropertySet("Errors");
++ const PropertySet* props = app->getPropertySet("Errors");
+
+- // First look for settings in the request map of the form pageError.
++ // If the externalParameters option isn't set, clear out the request field.
++ pair<bool,bool> externalParameters =
++ props ? props->getBool("externalParameters") : pair<bool,bool>(false,false);
++ if (!externalParameters.first || !externalParameters.second) {
++ tp.m_request = nullptr;
++ }
++
++ // Now look for settings in the request map of the form pageError.
+ try {
+ RequestMapper::Settings settings = request.getRequestSettings();
+ if (mderror)
+diff --git a/shibsp/handler/impl/AttributeCheckerHandler.cpp b/shibsp/handler/impl/AttributeCheckerHandler.cpp
+index 47dff9d..52f56fa 100644
+--- a/shibsp/handler/impl/AttributeCheckerHandler.cpp
++++ b/shibsp/handler/impl/AttributeCheckerHandler.cpp
+@@ -187,8 +187,16 @@ pair<bool,long> AttributeCheckerHandler::run(SPRequest& request, bool isHandler)
+
+ ifstream infile(m_template.c_str());
+ if (infile) {
+- TemplateParameters tp(nullptr, request.getApplication().getPropertySet("Errors"), session);
+- tp.m_request = &request;
++ const PropertySet* props = request.getApplication().getPropertySet("Errors");
++ TemplateParameters tp(nullptr, props, session);
++
++ // If the externalParameters option isn't set, don't populate the request field.
++ pair<bool,bool> externalParameters =
++ props ? props->getBool("externalParameters") : pair<bool,bool>(false,false);
++ if (externalParameters.first && externalParameters.second) {
++ tp.m_request = &request;
++ }
++
+ stringstream str;
+ XMLToolingConfig::getConfig().getTemplateEngine()->run(infile, str, tp);
+ if (m_flushSession && session) {
+diff --git a/shibsp/handler/impl/FormSessionInitiator.cpp b/shibsp/handler/impl/FormSessionInitiator.cpp
+index 522b31b..24dd77d 100644
+--- a/shibsp/handler/impl/FormSessionInitiator.cpp
++++ b/shibsp/handler/impl/FormSessionInitiator.cpp
+@@ -123,8 +123,18 @@ pair<bool,long> FormSessionInitiator::run(SPRequest& request, string& entityID,
+ ifstream infile(XMLToolingConfig::getConfig().getPathResolver()->resolve(fname, PathResolver::XMLTOOLING_CFG_FILE).c_str());
+ if (!infile)
+ throw ConfigurationException("Unable to access HTML template ($1).", params(1, m_template));
++
++ const PropertySet* props = app.getPropertySet("Errors");
++
+ TemplateParameters tp;
+- tp.m_request = &request;
++
++ // If the externalParameters option isn't set, don't populate the request field.
++ pair<bool,bool> externalParameters =
++ props ? props->getBool("externalParameters") : pair<bool,bool>(false,false);
++ if (externalParameters.first && externalParameters.second) {
++ tp.m_request = &request;
++ }
++
+ tp.setPropertySet(app.getPropertySet("Errors"));
+ tp.m_map["action"] = returnURL;
+ if (!target.empty())
+diff --git a/shibsp/handler/impl/LogoutHandler.cpp b/shibsp/handler/impl/LogoutHandler.cpp
+index 026be56..5673a12 100644
+--- a/shibsp/handler/impl/LogoutHandler.cpp
++++ b/shibsp/handler/impl/LogoutHandler.cpp
+@@ -63,6 +63,7 @@ pair<bool,long> LogoutHandler::sendLogoutPage(
+ {
+ string tname = string(type) + "Logout";
+ const PropertySet* props = application.getPropertySet("Errors");
++
+ pair<bool,const char*> prop = props ? props->getString(tname.c_str()) : pair<bool,const char*>(false,nullptr);
+ if (!prop.first) {
+ tname += ".html";
+@@ -76,7 +77,14 @@ pair<bool,long> LogoutHandler::sendLogoutPage(
+ if (!infile)
+ throw ConfigurationException("Unable to access $1 HTML template.", params(1,prop.second));
+ TemplateParameters tp;
+- tp.m_request = &request;
++
++ // If the externalParameters option isn't set, don't populate the request field.
++ pair<bool,bool> externalParameters =
++ props ? props->getBool("externalParameters") : pair<bool,bool>(false,false);
++ if (externalParameters.first && externalParameters.second) {
++ tp.m_request = &request;
++ }
++
+ tp.setPropertySet(props);
+ tp.m_map["logoutStatus"] = "Logout completed successfully."; // Backward compatibility.
+ stringstream str;
--
Thanks,
Feri
More information about the Pkg-shibboleth-devel
mailing list