これでいいのか javascript で HTML をエスケープ

jsonengine がシンプルな上になかなか優秀なので、お試ししてみました。

普通に CRUD する分にはまったく問題無し。快適。しかし、XSS な文字列を入力されちゃった場合の対処をどうするか。twitter でも話題に上っていたのを目にしました。

取りあえずサーバ側でエスケープするのが王道だろう、ということで修正してみました。

diff --git a/src/com/jsonengine/common/JEUtils.java b/src/com/jsonengine/common/JEUtils.java
index b2d2473..c545982 100644
--- a/src/com/jsonengine/common/JEUtils.java
+++ b/src/com/jsonengine/common/JEUtils.java
@@ -3,6 +3,7 @@ package com.jsonengine.common;
 import java.math.BigDecimal;

 import org.slim3.memcache.Memcache;
+import org.slim3.util.HtmlUtil;

 /**
  * Provides utility methods for jsonengine.
@@ -60,17 +61,18 @@ public class JEUtils {
         if (val == null) {
             return "";
         } else if (val instanceof String) {
-            return (String) val;
+            return HtmlUtil.escape((String) val);

これで、samples/bbs.html に

<script>alert("HOGE");</script>

を入力してみると、ほら大丈夫....え？！じゃない！

jQuery の使用経験はほとんどありませんので、よく分かっていないかも知れませんが、もしかしてエスケープしたタグ関連の文字列が復活しているのでしょうか？ 仕方無いので、取りあえず下記サイトを参考に

Escaping text with jQuery append? - Stack Overflow

$('<div></div>').text("<br>(" + getUserName(result[i]) + ") " + result[i].msg).appendTo('#messages');

javascript 側でもエスケープを掛けることで一応解決。う〜む。クライアント側で js を改変されたらヤバいよね、これ。

追記: ブラウザから直接クエリーしてみたら、エスケープした文字列が復活していました。

/_ah/admin/datastore?kind=JEDoc では確かに

[msg:_updatedAt:13:0.1280206640719, msg:msg:<script>alert("FUGA");</script>, msg:_createdBy:, msg:docType:msg, msg:_updatedBy:, msg:_createdAt:13:0.1280206640719]

にもかかわらず、/_q/msg では、

{"docType":"msg","msg":"<script>alert(\"FUGA\");</script>","_docId":"20HHlkxrczSB3aI3HV7QtQIzO5paI0hO","_updatedAt":1280206640719,"_createdAt":1280206640719,"_updatedBy":null,"_createdBy":null}

となっています。

ということで、原因は jQuery ではなく、jsonengine の方でしょうか？ さても、どこが問題箇所なのでしょう。JEDoc の docValues あたりかなぁ...

追記の追記: かなり勘違いしていました。

indexEntries をエスケープしていても、あまり意味ありません。データ本体である Blob な docValues をどうにかしないと。

追記の追記の追記: 出口で escape の網を張ってみました。

diff --git a/src/com/jsonengine/model/JEDoc.java b/src/com/jsonengine/model/JEDoc.java
index 42a0245..9fa2669 100644
--- a/src/com/jsonengine/model/JEDoc.java
+++ b/src/com/jsonengine/model/JEDoc.java
@@ -137,7 +138,10 @@ public class JEDoc implements Serializable {
      * @return JSON document
      */
     public String encodeJSON() {
-        return JSON.encode(getDocValues());
+        return HtmlUtil.escape(JSON.encode(getDocValues())).replaceAll(
+            """,
+            "\""");
+
     }

diff --git a/src/com/jsonengine/service/query/QueryService.java b/src/com/jsonengine/service/query/QueryService.java
index 45af946..49a7fdc 100644
--- a/src/com/jsonengine/service/query/QueryService.java
+++ b/src/com/jsonengine/service/query/QueryService.java
@@ -8,6 +8,7 @@ import net.arnx.jsonic.JSON;

 import org.slim3.datastore.Datastore;
 import org.slim3.datastore.ModelQuery;
+import org.slim3.util.HtmlUtil;

 import com.jsonengine.common.JEAccessDeniedException;
 import com.jsonengine.meta.JEDocMeta;
@@ -61,12 +62,12 @@ public class QueryService {
         // return the results in JSON
-        return JSON.encode(results);
+        return HtmlUtil.escape(JSON.encode(results)).replaceAll(""", "\""");
     }
 }

ただ、replaceAll とは、ちょっと無茶ぶりが過ぎるかなぁ、と。

docType からエスケープしたい対象だけ抜き出して処理する方が丁寧やも知れませんね。

追記の追記の追記の追記: 入口で escape の網を張ってみました。

やはりヤバげなデータを飲み込む、というのは何とも気持ち悪いので、入口規制を掛けてみました。

diff --git a/src/com/jsonengine/service/crud/CRUDRequest.java b/src/com/jsonengine/service/crud/CRUDRequest.java
index 021d657..05e84e1 100644
--- a/src/com/jsonengine/service/crud/CRUDRequest.java
+++ b/src/com/jsonengine/service/crud/CRUDRequest.java
@@ -5,6 +5,8 @@ import java.util.Map;

 import net.arnx.jsonic.JSON;

+import org.slim3.util.HtmlUtil;
+
 import com.jsonengine.common.JERequest;
 import com.jsonengine.model.JEDoc;

@@ -56,7 +58,18 @@ public class CRUDRequest extends JERequest {
         this.jsonDoc = jsonDoc;
         if (jsonDoc != null) {
             // decode jsonDoc and fill it into jsonMap
-            jsonMap = JSON.decode(jsonDoc, Map.class);
+            final Map<String, Object> temp = JSON.decode(jsonDoc, Map.class);
+            final String docType = temp.get("docType").toString();
+
+            if (temp.get(docType) instanceof String) {
+                final String escaped =
+                    HtmlUtil.escape(temp.get(docType).toString());
+                temp.put(docType, escaped);
+                jsonMap = temp;
+            } else {
+                jsonMap = JSON.decode(jsonDoc, Map.class);
+            }
+
             setDocId((String) jsonMap.get(JEDoc.PROP_NAME_DOCID));
         } else {
             jsonMap = null;

JSON という汎用性故に、どういうデータ形式が飛んでくるのか分かりませんので、美しくない型チェックを実施して String であれば、エスケープ処理を施すようにしてみました。

もっとイイ方法があるような気もするのですが...もう少し考えてみます。