From c7f2636b3efb803c5d7b7b91ff0a9da5339579fc Mon Sep 17 00:00:00 2001
From: tdelafosse <thomas.delafosse@xwiki.com>
Date: Wed, 12 Jun 2013 17:23:34 +0200
Subject: [PATCH] XWiki-9151 : XSS vulnerability in link syntax when using
 "path:"

---
 .../rendering/renderer/printer/XMLWikiPrinter.java |   68 ++++++++++++++++++--
 1 file changed, 64 insertions(+), 4 deletions(-)

diff --git a/xwiki-rendering-api/src/main/java/org/xwiki/rendering/renderer/printer/XMLWikiPrinter.java b/xwiki-rendering-api/src/main/java/org/xwiki/rendering/renderer/printer/XMLWikiPrinter.java
index 5d2ae5b..f2ab9e1 100644
--- a/xwiki-rendering-api/src/main/java/org/xwiki/rendering/renderer/printer/XMLWikiPrinter.java
+++ b/xwiki-rendering-api/src/main/java/org/xwiki/rendering/renderer/printer/XMLWikiPrinter.java
@@ -20,6 +20,8 @@
 package org.xwiki.rendering.renderer.printer;
 
 import java.io.IOException;
+import java.util.Arrays;
+import java.util.List;
 import java.util.Map;
 
 import org.dom4j.Element;
@@ -45,7 +47,18 @@
     protected WikiWriter wikiWriter;
 
     protected XMLWriter xmlWriter;
-
+    
+    /**
+     * List of authorized attributes.
+     */
+    private static final List<String> ATTRIBUTES_WHITELIST = 
+        Arrays.asList("alt", "class", "height", "id", "name", "rel", "scope", "style", "target", "title", "width");
+    
+    /**
+     * Attributes that should be authorized only if their value is safe.
+     */
+    private static final List<String> VULNERABLE_ATTRIBUTES = Arrays.asList("href", "src");
+    
     /**
      * @param printer the object to which to write the XHTML output to
      */
@@ -95,7 +108,10 @@ public void printXMLElement(String name, String[][] attributes)
 
         if (attributes != null && attributes.length > 0) {
             for (String[] entry : attributes) {
-                element.addAttribute(entry[0], entry[1]);
+                if(isAttributeClean(entry[0], entry[1])) {
+                    // We add this attribute if and only if it is safe.
+                    element.addAttribute(entry[0], entry[1]);
+                }
             }
         }
 
@@ -115,7 +131,9 @@ public void printXMLElement(String name, Map<String, String> attributes)
 
         if (attributes != null && !attributes.isEmpty()) {
             for (Map.Entry<String, String> entry : attributes.entrySet()) {
-                element.addAttribute(entry.getKey(), entry.getValue());
+                if(isAttributeClean(entry.getKey(), entry.getValue())) {
+                  element.addAttribute(entry.getKey(), entry.getValue());
+                }
             }
         }
 
@@ -156,7 +174,7 @@ public void printXMLStartElement(String name, Map<String, String> attributes)
     public void printXMLStartElement(String name, Attributes attributes)
     {
         try {
-            this.xmlWriter.startElement("", name, name, attributes);
+            this.xmlWriter.startElement("", name, name, cleanAttributes(attributes));
         } catch (SAXException e) {
             // TODO: add error log here
         }
@@ -289,4 +307,46 @@ private Attributes createAttributes(Map<String, String> parameters)
 
         return attributes;
     }
+    
+    /**
+     * Clean attributes to prevent XSS.
+     * 
+     * @param attributes Attributes to clean
+     * @return clean attributes 
+     */
+    private Attributes cleanAttributes(Attributes attributes)
+    {
+        AttributesImpl cleanAttributes = new AttributesImpl();
+        int length = attributes.getLength();
+        for (int i=0; i<length; i++) {
+            String qName = attributes.getQName(i);
+            String value = attributes.getValue(i);
+            if (isAttributeClean(qName, value)) {
+                cleanAttributes.addAttribute(attributes.getURI(i), attributes.getLocalName(i), qName, attributes.getType(i), value);
+            }
+        }
+        return cleanAttributes;
+    }
+
+    /**
+     * Determine whether an attribute is safe or not
+     * 
+     * @param key Name of the attribute
+     * @param value Value of the attribute
+     * @return true if the attribute is in the attributes whitelist or if its value is safe 
+     */
+    private boolean isAttributeClean(String key, String value)
+    {
+        // Let's trim the attribute value to make sure that leading whitespaces won't create any issue.
+        value = value.trim();
+        if (ATTRIBUTES_WHITELIST.contains(key)) {
+            return true;
+        } else if (VULNERABLE_ATTRIBUTES.contains(key)) {
+            if(value.startsWith("/") || value.startsWith("http") || value.startsWith("www") 
+                || value.startsWith("mailto") || value.startsWith("#") || value.startsWith("file")) {
+                return true;
+            }
+        }
+        return false;
+    }
 }
-- 
1.7.10.4