Description: Fix RSS Feeds Cross Domain Scripting Vulnerability
CVE-2009-4102 Cross Domain Scripting vulnerability. Don't trust HTML in titles,
descriptions. Don't allow 'strange' (i.e. javascript:, data:) URLs in Links.
CVE-2006-4712 (Regression), some of the old test cases no longer passed due to
problem with htmlToText.
Bug-Debian: http://bugs.debian.org/559267
Author: Alan Woodland <awoodland@debian.org>
--- sage-1.4.3.orig/chrome/sage.jar!/content/createhtml.js
+++ sage-1.4.3/chrome/sage.jar!/content/createhtml.js
@@ -133,29 +133,37 @@ var CreateHTML = {
switch (s) {
case "**TITLE**":
- return this.entityEncode(feed.getTitle());
+ // Entity encode is correct here - we shouldn't let any HTML through
+ return this.entityEncode(SageUtils.htmlToText(feed.getTitle()));
case "**LINK**":
- return feed.getLink();
+ // Partial fix for CVE-2009-4102
+ // Clean href is correct here - there is HTML in what gets returned by getLink, but it's all Sage generated and anything which can break out of it should be escaped
+ return this.cleanHref(feed.getLink());
break;
case "**AUTHOR**":
if (feed.hasAuthor()) {
- return "<div class=\"feed-author\">" + this.entityEncode(feed.getAuthor()) + "</div>";
+ // Entity encode is correct - we don't want any HTML back from this
+ return "<div class=\"feed-author\">" + this.entityEncode(SageUtils.htmlToText(feed.getAuthor())) + "</div>";
}
return "";
case "**DESCRIPTION**":
if (feed.hasDescription()) {
- return feed.getDescription();
+ // Entity encode call is Partial fix for CVE-2009-4102
+ // EntityEncode + htmlToText is too strict really, but not broken. Ideally we should honour the 'display HTML' preference here.
+ return this.entityEncode(SageUtils.htmlToText(feed.getDescription()));
}
return "";
/*
case "**LOGOLINK**":
+ // need to be sure we can't escape the href="..." part this gets enclosed in
return feed.getLogo().link;
case "**LOGOALT**":
+ // need to be sure we can't escape the alt="..."
return feed.getLogo().alt;
case "**COPYRIGHT**":
@@ -185,6 +193,7 @@ var CreateHTML = {
return "";
*/
case "**ITEMS**":
+ // Correct - getItemsHtml is already escaped/quoted internally
return this.getItemsHtml(feed);
}
@@ -199,6 +208,7 @@ var CreateHTML = {
}
var sb = [];
for (var i = 0; i < feed.getItemCount(); i++) {
+ // Correct - already quoted/escaped
sb.push(this.getItemHtml(feed, feed.getItem(i), i));
}
return sb.join("");
@@ -216,20 +226,26 @@ var CreateHTML = {
return i +1;
case "**LINK**":
- return item.getLink();
+ // Partial fix for CVE-2009-4102
+ // Correct - be careful of breaking out of the href="..." though
+ return this.cleanHref(item.getLink());
case "**TITLE**":
if (item.hasTitle()) {
- return this.entityEncode(item.getTitle());
+ // correct - this doesn't let any HTML through
+ return this.entityEncode(SageUtils.htmlToText(item.getTitle()));
} else if (item.getTitle()) {
- return this.entityEncode(item.getTitle());
+ // correct - no HTML through
+ return this.entityEncode(SageUtils.htmlToText(item.getTitle()));
} else {
+ // No HTML here eitther, but it's not input anyway
return this.entityEncode(strRes.GetStringFromName("feed_item_no_title"));
}
case "**AUTHOR**":
if (item.hasAuthor()) {
- return "<div class=\"item-author\">" + this.entityEncode(item.getAuthor()) + "</div>";
+ // Correct - no HTML permitted here
+ return "<div class=\"item-author\">" + this.entityEncode(SageUtils.htmlToText(item.getAuthor())) + "</div>";
}
return "";
@@ -239,10 +255,13 @@ var CreateHTML = {
var ds;
if (allowEContent) {
this.filterHtmlHandler.clear();
+ // Correct provided simpleHtmlParser works
this.simpleHtmlParser.parse(item.getContent());
ds = this.filterHtmlHandler.toString();
} else {
- ds = SageUtils.htmlToText(item.getContent());
+ // Entity encode call is fix for regression from CVE-2006-4712
+ // Correct
+ ds = this.entityEncode(SageUtils.htmlToText(item.getContent()));
}
return "<div class=\"item-desc\">" + ds + "</div>";
}
@@ -253,6 +272,7 @@ var CreateHTML = {
var twelveHourClock = SageUtils.getSagePrefValue(SageUtils.PREF_TWELVE_HOUR_CLOCK);
var formatter = Components.classes["@sage.mozdev.org/sage/dateformatter;1"].getService(Components.interfaces.sageIDateFormatter);
formatter.setFormat(formatter.FORMAT_LONG, formatter.ABBREVIATED_FALSE, twelveHourClock ? formatter.CLOCK_12HOUR : formatter.CLOCK_24HOUR);
+ // Correct provided formatDate isn't buggy
var dateString = formatter.formatDate(item.getPubDate());
return "<div class=\"item-pubDate\">" + dateString + "</div>";
}
@@ -260,6 +280,7 @@ var CreateHTML = {
case "**ENCLOSURE**":
if (item.hasEnclosure()) {
+ // ??
var enc = item.getEnclosure();
function createDescriptionFromURL(url) {
var array = url.split("/");
@@ -270,7 +291,8 @@ var CreateHTML = {
return description;
}
return "<div class=\"item-enclosure\">" +
- "<a href=\"" + enc.getLink() + "\" title=\"" +
+ // correct
+ "<a href=\"" + cleanHref(enc.getLink()) + "\" title=\"" +
createDescriptionFromURL(enc.getLink()) +
"\"><img src=\"" +
(enc.hasMimeType() ?
@@ -291,6 +313,33 @@ var CreateHTML = {
return dirService.get(aProp, Components.interfaces.nsILocalFile);
},
+ // Partial fix for CVE-2009-4102
+ cleanHref: function(aUrl)
+ {
+ // We only want to allow http, ftp, news and mailto before :
+ var ltype = aUrl.split(":")[0];
+ // Make it greedy so there cannot be any surplus :'s left after filtering
+ // This was an error in my original patch
+ aUrl = aUrl.replace(/^.*:/, "");
+ switch(ltype.toLowerCase())
+ {
+ case "http":
+ aUrl = ltype + ":" + aUrl;
+ break;
+ case "nntp":
+ aUrl = ltype + ":" + aUrl;
+ break;
+ case "mailto":
+ aUrl = ltype + ":" + aUrl;
+ break;
+ case "ftp":
+ aUrl = ltype + ":" + aUrl;
+ break;
+ }
+ // Did I miss some safe ones?
+ return aUrl
+ },
+
entityEncode: function(aStr)
{
function replacechar(match) {
--- sage-1.4.3.orig/chrome/sage.jar!/content/filterhtmlhandler.js
+++ sage-1.4.3/chrome/sage.jar!/content/filterhtmlhandler.js
@@ -139,7 +139,7 @@ FilterHtmlHandler.prototype = {
} else if (nl.substr(0,2) == "on") {
//noop
} else if ((nl == "href" || nl == "src" || nl == "data" || nl == "codebase") &&
- /^javascript\:/i.test(vl)) {
+ /^javascript\:/i.test(vl) && /^data\:/i.test(vl)) {
//noop
} else if (tl == "img" && nl == "src" &&
vl.substring(0, 7) == "mailto:") {