Details

    • Type: Sub-task Sub-task
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.1-BETA5
    • Component/s: Javascript
    • Labels:
      None
    1. fix-346-bug.patch
      24 kB
      Reema Sardana
    2. split-sanitizer-and-domita-minified.patch
      1 kB
      Jasvir Nagra

      Activity

      Hide
      Zhen Wang added a comment -

      This function is briefly mentioned in the v0.8 release notes (http://code.google.com/apis/opensocial/docs/releasenotes.html) but not defined in the API reference (http://code.google.com/apis/opensocial/docs/0.8/reference/gadgets/#gadgets.util).

      It's also very vague how HTML should be sanitized by this function. I assume it's supposed to strip all JavaScript from the input. Correct me if I'm wrong.

      Show
      Zhen Wang added a comment - This function is briefly mentioned in the v0.8 release notes ( http://code.google.com/apis/opensocial/docs/releasenotes.html ) but not defined in the API reference ( http://code.google.com/apis/opensocial/docs/0.8/reference/gadgets/#gadgets.util ). It's also very vague how HTML should be sanitized by this function. I assume it's supposed to strip all JavaScript from the input. Correct me if I'm wrong.
      Hide
      Kevin Brown added a comment -

      http://opensocial-resources.googlecode.com/svn/spec/0.8/gadgets/util.js is the canonical reference (it's also fairly vague, but at least it's defined). This is what's linked from opensocial.org, and contains what was agreed to on the spec discussion list.

      As to why code.google.com/apis/opensocial/... doesn't match what's on opensocial.org, I can't say. Dan Peterson would probably be able to straighten it out.

      Show
      Kevin Brown added a comment - http://opensocial-resources.googlecode.com/svn/spec/0.8/gadgets/util.js is the canonical reference (it's also fairly vague, but at least it's defined). This is what's linked from opensocial.org, and contains what was agreed to on the spec discussion list. As to why code.google.com/apis/opensocial/... doesn't match what's on opensocial.org, I can't say. Dan Peterson would probably be able to straighten it out.
      Hide
      Kevin Brown added a comment -

      Brian – do you want to take this one? It was your proposal after all

      Show
      Kevin Brown added a comment - Brian – do you want to take this one? It was your proposal after all
      Hide
      Brian Eaton added a comment -

      Nope, I'm tapped out on OAuth and the gadget security token right now. Talk to the Caja guys, maybe?

      Show
      Brian Eaton added a comment - Nope, I'm tapped out on OAuth and the gadget security token right now. Talk to the Caja guys, maybe?
      Hide
      Reema Sardana added a comment -

      Stolen from the html sanitizer in the caja code written by Mike Samuel.

      Show
      Reema Sardana added a comment - Stolen from the html sanitizer in the caja code written by Mike Samuel.
      Hide
      Brian Eaton added a comment -

      Thanks for the patch Reema.

      We should find a way to reuse the html_sanitize function without cutting and pasting the code into util.js. The Caja code in Shindig has recently been redone, but I think you can reference features/caja/feature.xml for an example of how to pull source out of caja. Something like this, maybe?

      <script src="res:///com/google/caja/plugin/html-sanitizer.js"></script>

      The bigger issue is the lack of URL sanitization. If a gadget author calls sanitizeHtml, they should be guaranteed that the resulting HTML snippet contains no script. Something like <a href="javascript:..."> should not be possible.

      Would you be willing to work on the URL sanitization part while I try to track down someone who with a long term vision about how Caja and Shindig should share code to help with the other piece?

      Show
      Brian Eaton added a comment - Thanks for the patch Reema. We should find a way to reuse the html_sanitize function without cutting and pasting the code into util.js. The Caja code in Shindig has recently been redone, but I think you can reference features/caja/feature.xml for an example of how to pull source out of caja. Something like this, maybe? <script src="res:///com/google/caja/plugin/html-sanitizer.js"></script> The bigger issue is the lack of URL sanitization. If a gadget author calls sanitizeHtml, they should be guaranteed that the resulting HTML snippet contains no script. Something like <a href="javascript:..."> should not be possible. Would you be willing to work on the URL sanitization part while I try to track down someone who with a long term vision about how Caja and Shindig should share code to help with the other piece?
      Hide
      Reema Sardana added a comment -

      That sounds good. I will keep you updated with my progress on that.

      Show
      Reema Sardana added a comment - That sounds good. I will keep you updated with my progress on that.
      Hide
      Jasvir Nagra added a comment -

      Attached is a small patch that:
      (1) pulls the latest version of caja (which has html-santizer and caja split up)
      (2) includes both domita and html-sanitizer in feature.xml

      Once the changes to gadget.util have been made that include html-sanitizer-minified, (2) can be removed.

      Show
      Jasvir Nagra added a comment - Attached is a small patch that: (1) pulls the latest version of caja (which has html-santizer and caja split up) (2) includes both domita and html-sanitizer in feature.xml Once the changes to gadget.util have been made that include html-sanitizer-minified, (2) can be removed.
      Hide
      Brian Eaton added a comment -

      Thanks Jas. I've submitted your patch, tweaked slightly to make gadgets.util depend on the HTML sanitizer.

      Show
      Brian Eaton added a comment - Thanks Jas. I've submitted your patch, tweaked slightly to make gadgets.util depend on the HTML sanitizer.
      Hide
      Kevin Brown added a comment -

      This is not an acceptable way to implement the sanitizer, because it only works for the Java build.

      Show
      Kevin Brown added a comment - This is not an acceptable way to implement the sanitizer, because it only works for the Java build.
      Hide
      Paul Lindner added a comment -

      Hi,
      Right now we have this construct in features:

      <script src="res:///com/google/caja/plugin/html-sanitizer.js"></script>

      Can we either replace this with a direct link to

      http://google-caja.googlecode.com/svn/trunk/src/com/google/caja/plugin/html-sanitizer.js

      I don't like it since it's just off trunk, but it would at least get the PHP version working for this feature.

      Show
      Paul Lindner added a comment - Hi, Right now we have this construct in features: <script src="res:///com/google/caja/plugin/html-sanitizer.js"></script> Can we either replace this with a direct link to http://google-caja.googlecode.com/svn/trunk/src/com/google/caja/plugin/html-sanitizer.js I don't like it since it's just off trunk, but it would at least get the PHP version working for this feature.
      Hide
      Paul Lindner added a comment -

      ... or have the PHP code rewrite res://com/google/caja/ to
      http://google-caja.googlecode.com/svn/trunk/src/com/google/caja/

      Show
      Paul Lindner added a comment - ... or have the PHP code rewrite res://com/google/caja/ to http://google-caja.googlecode.com/svn/trunk/src/com/google/caja/
      Hide
      Paul Lindner added a comment -

      How about this?

      diff --git a/php/src/gadgets/GadgetFeatureRegistry.php b/php/src/gadgets/GadgetFeatureRegistry.php
      index 0f77901..f8c10e7 100644
      — a/php/src/gadgets/GadgetFeatureRegistry.php
      +++ b/php/src/gadgets/GadgetFeatureRegistry.php
      @@ -208,6 +208,12 @@ class GadgetFeatureRegistry

      { $content = (string)$script; }

      else {
      $content = trim($attributes['src']);
      +
      + // Make html-santitization work.
      + if ($content == 'res://com/google/caja/plugin/html-sanitizer.js')

      { + $content= 'http://google-caja.googlecode.com/svn/trunk/src/com/google/caja/plugin/html-sanitizer.js'; + }

      +
      if (strtolower(substr($content, 0, strlen("http://"))) == "http://" || strtolower(substr($content, 0, strlen("https://"))) == "https://")

      { $type = 'URL'; }

      else {

      Show
      Paul Lindner added a comment - How about this? diff --git a/php/src/gadgets/GadgetFeatureRegistry.php b/php/src/gadgets/GadgetFeatureRegistry.php index 0f77901..f8c10e7 100644 — a/php/src/gadgets/GadgetFeatureRegistry.php +++ b/php/src/gadgets/GadgetFeatureRegistry.php @@ -208,6 +208,12 @@ class GadgetFeatureRegistry { $content = (string)$script; } else { $content = trim($attributes ['src'] ); + + // Make html-santitization work. + if ($content == 'res://com/google/caja/plugin/html-sanitizer.js') { + $content= 'http://google-caja.googlecode.com/svn/trunk/src/com/google/caja/plugin/html-sanitizer.js'; + } + if (strtolower(substr($content, 0, strlen("http://"))) == "http://" || strtolower(substr($content, 0, strlen("https://"))) == "https://") { $type = 'URL'; } else {
      Hide
      Paul Lindner added a comment -

      patch applied, let's move on..

      Show
      Paul Lindner added a comment - patch applied, let's move on..

        People

        • Assignee:
          Chris Chabot
          Reporter:
          Kevin Brown
        • Votes:
          0 Vote for this issue
          Watchers:
          0 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development