Markdown: Fix Do not XSS defence

- Failed to defence XSS attack after markdown rendering on server - XSS attack : https://www.owasp.org/index.php/Cross-site_Scripting_(XSS) - Fix : escape string to html special character and using xss.js on server rendering
naver · Mar 11, 2015 · 39fab71 · 39fab71
1 parent 11b12b7
commit 39fab71
Show file tree

Hide file tree

Showing 8 changed files with 22 additions and 11 deletions.
diff --git a/app/utils/Markdown.java b/app/utils/Markdown.java
@@ -32,6 +32,7 @@
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.Reader;
+import java.lang.System;
 import java.net.URI;
 import java.net.URISyntaxException;
 
@@ -122,8 +123,7 @@ private static String checkReferrer(String source) {
     private static String sanitize(String source) {
         try {
             Object filter = engine.eval("new Filter();");
-            Object sanitize = ((Invocable) engine).invokeMethod(filter, "sanitize", source);
-            return new JSInvocable((Invocable) engine, sanitize).invoke("xss");
+            return (String) ((Invocable) engine).invokeMethod(filter, "defence", source);
         } catch (Exception ex) {
             throw new RuntimeException(ex);
         }

diff --git a/app/views/common/markdown.scala.html b/app/views/common/markdown.scala.html
@@ -23,7 +23,6 @@
 <link rel="stylesheet" type="text/css" href="@routes.Assets.at("javascripts/lib/highlight/styles/default.css")" />
 <script type="text/javascript" src="@routes.Assets.at("javascripts/lib/highlight/highlight.pack.js")"></script>
 <script type="text/javascript" src="@routes.Assets.at("javascripts/lib/marked.js")"></script>
-<script type="text/javascript" src="@routes.Assets.at("javascripts/lib/xss.js")"></script>
 <script type="text/javascript">
 $(document).ready(function(){
     var htOptions = {};

diff --git a/app/views/common/scripts.scala.html b/app/views/common/scripts.scala.html
@@ -74,6 +74,7 @@
 <script type="text/javascript" src="@routes.Assets.at("javascripts/lib/humanize.js")"></script>
 <script type="text/javascript" src="@routes.Assets.at("javascripts/lib/validate.js")"></script>
 <script type="text/javascript" src="@routes.Assets.at("javascripts/lib/spin.js")"></script>
+<script type="text/javascript" src="@routes.Assets.at("javascripts/lib/xss.js")"></script>
 <script type="text/javascript" src="@routes.Assets.at("javascripts/common/yobi.Attachments.js")"></script>
 <script type="text/javascript" src="@routes.Assets.at("javascripts/common/yobi.Files.js")"></script>
 <script type="text/javascript" src="@routes.Assets.at("javascripts/common/yobi.Mention.js")"></script>
@@ -153,8 +154,8 @@
                 }
 
                 welTarget.popover({
-                    "title"    : htData.title,
-                    "content"  : htData.body,
+                    "title"    : htData.title.replace('<', '&lt;'),
+                    "content"  : $yobi.xssClean(htData.body),
                     "html"     : true,
                     "placement": "right",
                     "trigger"  : "hover"

diff --git a/public/javascripts/common/yobi.Common.js b/public/javascripts/common/yobi.Common.js
@@ -412,6 +412,12 @@ $yobi = yobi.Common = (function(){
         return null;
     }
 
+    function xssClean(str) {
+      var filter = new Filter();
+
+      return filter.defence(str);
+    }
+
     /* public Interface */
     return {
         "setScriptPath"   : setScriptPath,
@@ -430,7 +436,8 @@ $yobi = yobi.Common = (function(){
         "nl2br"     : nl2br,
         "tmpl"      : processTpl,
         "htmlspecialchars": htmlspecialchars,
-        "isImageFile": isImageFile
+        "isImageFile": isImageFile,
+        "xssClean" : xssClean
     };
 })();
 
diff --git a/public/javascripts/common/yobi.Markdown.js b/public/javascripts/common/yobi.Markdown.js
@@ -38,7 +38,6 @@ yobi.Markdown = (function(htOptions){
      * @param {Hash Table} htOptions
      */
     function _initVar(htOptions){
-        htVar.htFilter = new Filter();
         htVar.sMarkdownRendererUrl = htOptions.sMarkdownRendererUrl;
 
         htVar.htMarkedOption = {
@@ -69,7 +68,7 @@ yobi.Markdown = (function(htOptions){
      * @return {String}
      */
     function _renderMarkdown(sText) {
-        return htVar.htFilter.sanitize(marked(sText, htVar.htMarkedOption)).xss();
+        return $yobi.xssClean(marked(sText, htVar.htMarkedOption));
     }
 
     /**
@@ -125,7 +124,6 @@ yobi.Markdown = (function(htOptions){
         $(elContainer).on("click", 'a[data-mode="preview"]', function(weEvt){
             var welPreview = $(weEvt.delegateTarget).find("div.markdown-preview");
             var sContentBody = welTextarea.val();
-            welPreview.html(sContentBody);
 
             _replaceAutoLink(welPreview, sContentBody);
 

diff --git a/public/javascripts/common/yobi.ui.Select2.js b/public/javascripts/common/yobi.ui.Select2.js
@@ -80,7 +80,7 @@
                                     || '<div title="[${stateLabel}] ${name}">${name}</div>';
 
                 var formattedResult = $yobi.tmpl(tplMilestoneItem, {
-                    "name" : itemObject.text.trim(),
+                    "name" : itemObject.text.trim().replace('<', '&lt;'),
                     "state": milestoneState,
                     "stateLabel": milestoneStateLabel
                 });

diff --git a/public/javascripts/lib/xss.js b/public/javascripts/lib/xss.js
@@ -920,6 +920,10 @@
         return this.str;
     }
 
+    Filter.prototype.defence = function(str) {
+        return this.sanitize(str).xss();
+    }
+
     Filter.prototype.entityDecode = function() {
         this.modify(decode(this.str));
         return this.str;

diff --git a/public/javascripts/service/yobi.code.Browser.js b/public/javascripts/service/yobi.code.Browser.js
@@ -515,7 +515,9 @@
                 aCrumbs.push('<a href="' + sLink + '">' + sName + '</a>');
             });
 
-            htElement.welBreadCrumbs.html(aCrumbs.join(""));
+            var breadcrumb = $yobi.xssClean(aCrumbs.join(""));
+
+            htElement.welBreadCrumbs.html(breadcrumb);
         }
 
         _init(htOptions || {});