Permalink
Browse files

Suppress CSP errors when the app is of type 'web' (bug 841840)

  • Loading branch information...
mattbasta committed Feb 15, 2013
1 parent 1762878 commit 16d2cab698621083e1e92e2123cbd280a9e300e7
View
@@ -2,20 +2,34 @@
CSP_INFO = "https://developer.mozilla.org/en-US/docs/Security/CSP"
MESSAGE_TITLE = "CSP Violation Detected"
-MESSAGE_DESCRIPTION = ["It appears that your code may be performing an action "
- "which violates the CSP (content security policy) for "
- "privileged apps.",
- "You can find more information about what is and is "
- "not allowed by the CSP on the Mozilla Developers "
- "website. %s" % CSP_INFO]
+MESSAGE_GENERAL_DESC = ("You can find more information about what is and is "
+ "not allowed by the CSP on the Mozilla Developers "
+ "website. %s" % CSP_INFO)
+MESSAGE_DESC = ["It appears that your code may be performing an action which "
+ "violates the CSP (content security policy) for privileged "
+ "apps.", MESSAGE_GENERAL_DESC]
+MESSAGE_DESC_WEB = ["An action that you're performing violates the CSP "
+ "(content security policy). While this does not affect "
+ "your app, if you decide to add permissions to your app "
+ "in the future, you will be unable to do so until this "
+ "problem is corrected. It is highly recommended that "
+ "you remedy this.", MESSAGE_GENERAL_DESC]
def warn(err, filename, line, column, context, violation_type="default",
severity="warning"):
+
+ app_type = err.get_resource("app_type")
+
+ if app_type == "web":
+ severity = "warning"
+
params = dict(err_id=("csp", violation_type),
- description=MESSAGE_DESCRIPTION,
+ description=MESSAGE_DESC_WEB if app_type == "web" else
+ MESSAGE_DESC,
filename=filename,
line=line,
column=column,
context=context)
params[severity] = MESSAGE_TITLE
+
getattr(err, severity)(**params)
@@ -7,7 +7,7 @@
from appvalidator.contextgenerator import ContextGenerator
from appvalidator.constants import *
from appvalidator.csp import warn as message_csp
-from patchedhtmlparser import PatchedHTMLParser
+from patchedhtmlparser import htmlparser, PatchedHTMLParser
DEBUG = False
@@ -18,11 +18,11 @@
TAG_NOT_OPENED = "Tag (%s) being closed before it is opened."
REMOTE_URL_PATTERN = re.compile("((ht|f)tps?:)?//")
-DOM_MUTATION_HANDLERS = (
+DOM_MUTATION_HANDLERS = set([
"ondomattrmodified", "ondomattributenamechanged",
"ondomcharacterdatamodified", "ondomelementnamechanged",
"ondomnodeinserted", "ondomnodeinsertedintodocument", "ondomnoderemoved",
- "ondomnoderemovedfromdocument", "ondomsubtreemodified", )
+ "ondomnoderemovedfromdocument", "ondomsubtreemodified", ])
class MarkupParser(PatchedHTMLParser):
@@ -44,7 +44,7 @@ def __init__(self, err, strict=True, debug=False):
self.reported = set()
- def process(self, filename, data, extension="xul"):
+ def process(self, filename, data, extension="html"):
"""Processes data by splitting it into individual lines, then
incrementally feeding each line into the parser, increasing the
value of the line number with each line."""
@@ -107,7 +107,7 @@ def _feed_parser(self, line):
# There's no recovering from a unicode error here. We've got the
# unicodehelper; if that doesn't help us, nothing will.
return
- except Exception as inst:
+ except htmlparser.HTMLParseError as inst:
if DEBUG: # pragma: no cover
print self.xml_state, inst
@@ -153,7 +153,6 @@ def handle_starttag(self, tag, attrs, self_closing=False):
# Normalize!
tag = tag.lower()
- orig_tag = tag
# Be extra sure it's not a self-closing tag.
if not self_closing:
@@ -162,59 +161,62 @@ def handle_starttag(self, tag, attrs, self_closing=False):
if DEBUG: # pragma: no cover
print "S: ", self.xml_state, tag, self_closing
- # Find CSS and JS attributes and handle their values like they
- # would otherwise be handled by the standard parser flow.
- for attr in attrs:
- attr_name, attr_value = attr[0].lower(), attr[1]
-
- # We don't care about valueless attributes.
- if attr_value is None:
- continue
-
- if attr_name == "style":
- csstester.test_css_snippet(self.err,
- self.filename,
- attr_value,
- self.line)
- elif attr_name.startswith("on"): # JS attribute
- # Warn about DOM mutation event handlers.
- if attr_name in DOM_MUTATION_HANDLERS:
- self.err.error(
- err_id=("testcases_markup_markuptester",
- "handle_starttag",
- "dom_manipulation_handler"),
- error="DOM Mutation Events Prohibited",
- description="DOM mutation events are flagged because "
- "of their deprecated status, as well as "
- "their extreme inefficiency. Consider "
- "using a different event.",
- filename=self.filename,
- line=self.line,
- context=self.context)
-
- message_csp(err=self.err, filename=self.filename,
- line=self.line, column=None,
- context=self.context,
- violation_type="script_attribute",
- severity="error")
+ attr_dict = dict([(a[0].lower(), a[1]) for a in attrs if a[1]])
+
+ if "style" in attr_dict:
+ csstester.test_css_snippet(
+ self.err, self.filename, attr_dict["style"], self.line)
+
+ script_attributes = dict(
+ (k, v) for k, v in attr_dict.iteritems() if k.startswith("on"))
+ if script_attributes:
+ if any(k in DOM_MUTATION_HANDLERS for k in script_attributes):
+ self.err.error(
+ err_id=("testcases_markup_markuptester",
+ "handle_starttag", "dom_manipulation_handler"),
+ error="DOM Mutation Events Prohibited",
+ description="DOM mutation events are flagged because of "
+ "their deprecated status, as well as thier "
+ "extreme inefficiency. Consider using a "
+ "different event.",
+ filename=self.filename,
+ line=self.line,
+ context=self.context)
- elif attr_name == "src" and tag == "script":
- if not self._is_url_local(attr_value):
- message_csp(err=self.err, filename=self.filename,
- line=self.line, column=None,
- context=self.context,
- violation_type="remote_script",
- severity="error")
+ message_csp(err=self.err, filename=self.filename,
+ line=self.line, column=None, context=self.context,
+ violation_type="script_attribute", severity="error")
# When the dev forgets their <!-- --> on a script tag, bad
# things happen.
if "script" in self.xml_state and tag != "script":
self._save_to_buffer("<" + tag + self._format_args(attrs) + ">")
return
- self.xml_state.append(orig_tag)
+ elif (tag == "script" and
+ ("type" not in attr_dict or
+ any(a[0] == "type" and "javascript" in a[1].lower() for
+ a in attrs))):
+ # Inspect scripts which either have no type or have a type which
+ # is JS.
+
+ if "src" not in attr_dict:
+ # CSP warnings for inline scripts
+ message_csp(err=self.err, filename=self.filename,
+ line=self.line, column=None,
+ context=self.context,
+ violation_type="inline_script",
+ severity="error")
+
+ elif not self._is_url_local(attr_dict.get("src", "")):
+ # If there's a remote SRC, then that's a CSP violation.
+ message_csp(err=self.err, filename=self.filename,
+ line=self.line, column=None, context=self.context,
+ violation_type="remote_script", severity="error")
+
+ self.xml_state.append(tag)
self.xml_line_stack.append(self.line)
- self.xml_buffer.append(unicode(""))
+ self.xml_buffer.append(u"")
def handle_endtag(self, tag):
@@ -303,16 +305,9 @@ def handle_endtag(self, tag):
data_buffer = data_buffer.strip()
# Perform analysis on collected data.
- if data_buffer:
- if tag == "script":
- message_csp(err=self.err, filename=self.filename,
- line=self.line, column=None,
- context=self.context,
- violation_type="inline_script",
- severity="error")
- elif tag == "style":
- csstester.test_css_file(self.err, self.filename, data_buffer,
- old_line)
+ if data_buffer and tag == "style":
+ csstester.test_css_file(self.err, self.filename, data_buffer,
+ old_line)
def handle_data(self, data):
self._save_to_buffer(data)
@@ -31,6 +31,8 @@ def test_app_manifest(err, package):
webapp = detect_webapp_string(err, package.read("manifest.webapp"))
err.save_resource("manifest", webapp)
+ if webapp:
+ err.save_resource("app_type", str(webapp.get("type", "web")).lower())
@register_test(tier=2)
@@ -40,7 +42,7 @@ def test_permissions(err, package):
not err.get_resource("manifest")):
return
- app_type = err.get_resource("manifest").get("type", "web")
+ app_type = err.get_resource("app_type")
def error(permission):
err.error(
@@ -258,7 +260,7 @@ def test_icon(err, data, url, size):
"may contain invalid or corrupt data. Icons may "
"be only JPG or PNG images.",
"%dpx icon (%s)" % (size, url)])
- else:
+ finally:
if gzf:
gzf.close()
else:
View
@@ -1,9 +1,58 @@
from nose.tools import eq_
-from js_helper import TestCase
+import appvalidator.testcases.markup.markuptester as markuptester
+from ..helper import TestCase
+from js_helper import TestCase as JSTestCase
-class TestCSP(TestCase):
+
+class TestCSPTags(TestCase):
+
+ def analyze(self, snippet, app_type="web"):
+ self.setup_err()
+ self.err.save_resource("app_type", app_type)
+ markuptester.MarkupParser(self.err, debug=True).process("", snippet)
+
+ def test_script_not_js(self):
+ markup = """
+ <script type="text/x-jquery-tmpl">foo</script>
+ """
+
+ self.analyze(markup)
+ self.assert_silent()
+
+ self.analyze(markup, "privileged")
+ self.assert_silent()
+
+ def test_script(self):
+ markup = """<script>foo</script>"""
+
+ self.analyze(markup)
+ self.assert_failed(with_warnings=True)
+
+ self.analyze(markup, "privileged")
+ self.assert_failed(with_errors=True)
+
+ def test_script_attrs(self):
+ markup = """<button onclick="foo();"></button>"""
+
+ self.analyze(markup)
+ self.assert_failed(with_warnings=True)
+
+ self.analyze(markup, "privileged")
+ self.assert_failed(with_errors=True)
+
+ def test_script_remote(self):
+ markup = """<script src="http://foo.bar/zip.js"></script>"""
+
+ self.analyze(markup)
+ self.assert_failed(with_warnings=True)
+
+ self.analyze(markup, "privileged")
+ self.assert_failed(with_errors=True)
+
+
+class TestCSP(JSTestCase):
def test_function(self):
self.run_script("var x = Function('foo');")
@@ -34,7 +83,7 @@ def test_setInterval_pass(self):
self.assert_silent()
-class TestCreateElement(TestCase):
+class TestCreateElement(JSTestCase):
def test_pass(self):
"Tests that createElement and createElementNS throw errors."
@@ -44,4 +44,3 @@ def test_remote_urls():
assert not t("UrL(/abc.def)")
assert t("url(HTTP://foo.bar/)")
-
@@ -22,6 +22,7 @@ def _test_xul_raw(data, path, should_fail=False, should_fail_csp=None,
extension = filename.split(".")[-1]
err = ErrorBundle()
+ err.save_resource("app_type", "certified")
if type_:
err.set_type(type_)
@@ -186,15 +187,15 @@ def test_script_attrs():
"""Test that script attributes are warned against."""
_test_xul_raw("""
- <foo><bar onzap="" /></foo>
+ <foo><bar onzap="asdf" /></foo>
""", "foo.xul", should_fail_csp=True)
def test_dom_mutation():
"""Test that DOM mutation events are warned against. This should fail both
the standard tests as well as the CSP tests."""
_test_xul_raw("""
- <foo><bar ondomattrmodified="" /></foo>
+ <foo><bar ondomattrmodified="asdf" /></foo>
""", "foo.xul", should_fail=True, should_fail_csp=True)
@@ -203,16 +204,15 @@ def test_proper_line_numbers():
"""Test that the proper line numbers are passed to test_js_snippet."""
err = _test_xul_raw("""<foo>
- <script>
+ <script> // This is line 2
eval("OWOWOWOWOWOWOWOW");
</script>
</foo>""", "foo.xul", should_fail_csp=True)
assert err.errors
error = err.errors[0]
eq_(error["file"], "foo.xul")
- # 4 because it detects the script when it gets closed.
- eq_(error["line"], 4)
+ eq_(error["line"], 2)
def test_script_scraping():
View
@@ -97,7 +97,7 @@ def setUp(self):
"type": "web",
}
- self.resources = []
+ self.resources = [("app_type", "web")]
def analyze(self):
"""Run the webapp tests on the file."""
Oops, something went wrong.

0 comments on commit 16d2cab

Please sign in to comment.