[geary/wip/728002-webkit2] Ensure strings passed to WebProcess via JS get escaped.



commit 1ce2026699f8df2a7175669cff8164abad41ba40
Author: Michael James Gratton <mike vee net>
Date:   Fri Jan 27 17:51:38 2017 +1100

    Ensure strings passed to WebProcess via JS get escaped.
    
    * src/client/components/client-web-view.vala (ClientWebView): Add ::call
      method that takes a Callable object. Replace all existing uses of
      ::run_javascript with that.
    
    * src/engine/util/util-js.vala (Geary.JS): Add ::escape_string function,
      Callable object for constructing safe, well-formed JS calls.

 src/client/components/client-web-view.vala         |   11 +++-
 src/client/composer/composer-web-view.vala         |   44 +++++-------
 .../conversation-viewer/conversation-web-view.vala |    8 +-
 src/engine/util/util-js.vala                       |   72 ++++++++++++++++++++
 4 files changed, 105 insertions(+), 30 deletions(-)
---
diff --git a/src/client/components/client-web-view.vala b/src/client/components/client-web-view.vala
index c0b07aa..c425a9b 100644
--- a/src/client/components/client-web-view.vala
+++ b/src/client/components/client-web-view.vala
@@ -330,7 +330,7 @@ public class ClientWebView : WebKit.WebView {
      * Load any remote images previously that were blocked.
      */
     public void load_remote_images() {
-        run_javascript.begin("geary.loadRemoteImages();", null);
+        this.call.begin(Geary.JS.callable("geary.loadRemoteImages"), null);
     }
 
     /**
@@ -360,6 +360,15 @@ public class ClientWebView : WebKit.WebView {
     }
 
     /**
+     * Invokes a {@link Geary.JS.Callable} on this web view.
+     */
+    protected async WebKit.JavascriptResult call(Geary.JS.Callable target,
+                                                 Cancellable? cancellable)
+    throws Error {
+        return yield run_javascript(target.to_string(), cancellable);
+    }
+
+    /**
      * Convenience function for registering and connecting JS messages.
      */
     protected inline void register_message_handler(string name,
diff --git a/src/client/composer/composer-web-view.vala b/src/client/composer/composer-web-view.vala
index 96dc7ae..d76479c 100644
--- a/src/client/composer/composer-web-view.vala
+++ b/src/client/composer/composer-web-view.vala
@@ -235,23 +235,21 @@ public class ComposerWebView : ClientWebView {
      */
     public void set_rich_text(bool enabled) {
         this.is_rich_text = enabled;
-        this.run_javascript.begin(
-            "geary.setRichText(%s);".printf(enabled ? "true" : "false"), null
-        );
+        this.call.begin(Geary.JS.callable("geary.setRichText").bool(enabled), null);
     }
 
     /**
      * Undoes the last edit operation.
      */
     public void undo() {
-        this.run_javascript.begin("geary.undo();", null);
+        this.call.begin(Geary.JS.callable("geary.undo"), null);
     }
 
     /**
      * Redoes the last undone edit operation.
      */
     public void redo() {
-        this.run_javascript.begin("geary.redo();", null);
+        this.call.begin(Geary.JS.callable("geary.redo"), null);
     }
 
     /**
@@ -357,16 +355,14 @@ public class ComposerWebView : ClientWebView {
      * will be inserted wrapping the selection.
      */
     public void insert_link(string href) {
-        this.run_javascript.begin(
-            "geary.insertLink(\"%s\");".printf(href), null
-        );
+        this.call.begin(Geary.JS.callable("geary.insertLink").string(href), null);
     }
 
     /**
      * Removes any A element at the current text cursor location.
      */
     public void delete_link() {
-        this.run_javascript.begin("geary.deleteLink();", null);
+        this.call.begin(Geary.JS.callable("geary.deleteLink"), null);
     }
 
     /**
@@ -386,23 +382,21 @@ public class ComposerWebView : ClientWebView {
      * Indents the line at the current text cursor location.
      */
     public void indent_line() {
-        this.run_javascript.begin("geary.indentLine();", null);
+        this.call.begin(Geary.JS.callable("geary.indentLine"), null);
     }
 
     /**
      * Updates the signature block if it has not been deleted.
      */
     public new void update_signature(string signature) {
-        this.run_javascript.begin(
-            "geary.updateSignature(\"%s\");".printf(signature), null
-        );
+        this.call.begin(Geary.JS.callable("geary.updateSignature").string(signature), null);
     }
 
     /**
      * Removes the quoted message (if any) from the composer.
      */
     public void delete_quoted_message() {
-        this.run_javascript.begin("geary.deleteQuotedMessage();", null);
+        this.call.begin(Geary.JS.callable("geary.deleteQuotedMessage"), null);
     }
 
     /**
@@ -411,8 +405,11 @@ public class ComposerWebView : ClientWebView {
     public async bool contains_attachment_keywords(string keyword_spec, string subject) {
         try {
             return WebKitUtil.to_bool(
-                yield run_javascript("geary.containsAttachmentKeyword(\"%s\", \"%s\");"
-                                     .printf(keyword_spec, subject), null)
+                yield call(
+                    Geary.JS.callable("geary.containsAttachmentKeyword")
+                    .string(keyword_spec)
+                    .string(subject),
+                    null)
                 );
         } catch (Error err) {
             debug("Error checking or attchment keywords: %s", err.message);
@@ -424,31 +421,28 @@ public class ComposerWebView : ClientWebView {
      * Converts plain text URLs in the editor content into links.
      */
     public async void linkify_content() throws Error {
-        yield run_javascript("geary.linkifyContent();", null);
+        this.call.begin(Geary.JS.callable("geary.linkifyContent"), null);
     }
 
     /**
      * Returns the editor content as an HTML string.
      */
     public async string? get_html() throws Error {
-        WebKit.JavascriptResult result = yield this.run_javascript(
-            "geary.getHtml();", null
+        return WebKitUtil.to_string(
+            yield call(Geary.JS.callable("geary.getHtml"), null)
         );
-        return WebKitUtil.to_string(result);
     }
 
     /**
      * Returns the editor text as RFC 3676 format=flowed text.
      */
     public async string? get_text() throws Error {
-        WebKit.JavascriptResult result = yield this.run_javascript(
-            "geary.getText();", null
-        );
-
         const int MAX_BREAKABLE_LEN = 72; // F=F recommended line limit
         const int MAX_UNBREAKABLE_LEN = 998; // SMTP line limit
 
-        string body_text = WebKitUtil.to_string(result);
+        string body_text = WebKitUtil.to_string(
+            yield call(Geary.JS.callable("geary.getText"), null)
+        );
         string[] lines = body_text.split("\n");
         GLib.StringBuilder flowed = new GLib.StringBuilder.sized(body_text.length);
         foreach (string line in lines) {
diff --git a/src/client/conversation-viewer/conversation-web-view.vala 
b/src/client/conversation-viewer/conversation-web-view.vala
index aac2686..6d927a7 100644
--- a/src/client/conversation-viewer/conversation-web-view.vala
+++ b/src/client/conversation-viewer/conversation-web-view.vala
@@ -64,8 +64,8 @@ public class ConversationWebView : ClientWebView {
      * Returns the current selection, for prefill as find text.
      */
     public async string? get_selection_for_find() throws Error{
-        WebKit.JavascriptResult result = yield this.run_javascript(
-            "geary.getSelectionForFind();", null
+        WebKit.JavascriptResult result = yield call(
+            Geary.JS.callable("geary.getSelectionForFind"), null
         );
         return WebKitUtil.to_string(result);
     }
@@ -74,8 +74,8 @@ public class ConversationWebView : ClientWebView {
      * Returns the current selection, for quoting in a message.
      */
     public async string? get_selection_for_quoting() throws Error {
-        WebKit.JavascriptResult result = yield this.run_javascript(
-            "geary.getSelectionForQuoting();", null
+        WebKit.JavascriptResult result = yield call(
+            Geary.JS.callable("geary.getSelectionForQuoting"), null
         );
         return WebKitUtil.to_string(result);
     }
diff --git a/src/engine/util/util-js.vala b/src/engine/util/util-js.vala
index af2fb26..e6f61f0 100644
--- a/src/engine/util/util-js.vala
+++ b/src/engine/util/util-js.vala
@@ -156,4 +156,76 @@ namespace Geary.JS {
         }
     }
 
+    /**
+     * Escapes a string so as to be safte to use as a JS string literal.
+     *
+     * This does not append opening or closing quotes.
+     */
+    public string escape_string(string value) {
+        const unichar[] RESERVED = {
+            '\x00', '\'', '"', '\\', '\n', '\r', '\v', '\t', '\b', '\f'
+        };
+        StringBuilder builder = new StringBuilder.sized(value.length);
+        for (int i = 0; i < value.length; i++) {
+            if (value.valid_char(i)) {
+                unichar c = value.get_char(i);
+                if (c in RESERVED) {
+                    builder.append_c('\\');
+                }
+                builder.append_unichar(c);
+            }
+        }
+        return (string) builder.data;
+    }
+
+    /**
+     * Convenience method for returning a new Callable instance.
+     */
+    public Callable callable(string base_name) {
+        return new Callable(base_name);
+    }
+
+    /**
+     * A class for constructing a well formed, safe, invokable JS call.
+     */
+    public class Callable {
+
+        private string base_name;
+        private string[] safe_args = new string[0];
+
+
+        public Callable(string base_name) {
+            this.base_name = base_name;
+        }
+
+        public string to_string() {
+            return base_name + "(" + global::string.joinv(",", safe_args) + ");";
+        }
+
+        public Callable string(string value) {
+            add_param("\"" + escape_string(value) + "\"");
+            return this;
+        }
+
+        public Callable double(double value) {
+            add_param(value.to_string());
+            return this;
+        }
+
+        public Callable int(int value) {
+            add_param(value.to_string());
+            return this;
+        }
+
+        public Callable bool(bool value) {
+            add_param(value ? "true" : "false");
+            return this;
+        }
+
+        private inline void add_param(string value) {
+            this.safe_args += value;
+        }
+
+    }
+
 }


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]