Suppress CSP errors when the app is of type 'web' (bug 841840)
This commit is contained in:
Matt Basta
Родитель
17628785cc
Коммит
16d2cab698
|
|
@ -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 . import csstester
|
|||
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 @@ SELF_CLOSING_TAGS = ("area", "base", "basefont", "br", "col", "frame", "hr",
|
|||
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 @@ class MarkupParser(PatchedHTMLParser):
|
|||
|
||||
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 @@ class MarkupParser(PatchedHTMLParser):
|
|||
# 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 @@ class MarkupParser(PatchedHTMLParser):
|
|||
|
||||
# Normalize!
|
||||
tag = tag.lower()
|
||||
orig_tag = tag
|
||||
|
||||
# Be extra sure it's not a self-closing tag.
|
||||
if not self_closing:
|
||||
|
|
@ -162,49 +161,31 @@ class MarkupParser(PatchedHTMLParser):
|
|||
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]
|
||||
attr_dict = dict([(a[0].lower(), a[1]) for a in attrs if a[1]])
|
||||
|
||||
# We don't care about valueless attributes.
|
||||
if attr_value is None:
|
||||
continue
|
||||
if "style" in attr_dict:
|
||||
csstester.test_css_snippet(
|
||||
self.err, self.filename, attr_dict["style"], self.line)
|
||||
|
||||
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)
|
||||
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)
|
||||
|
||||
message_csp(err=self.err, filename=self.filename,
|
||||
line=self.line, column=None,
|
||||
context=self.context,
|
||||
violation_type="script_attribute",
|
||||
severity="error")
|
||||
|
||||
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.
|
||||
|
|
@ -212,9 +193,30 @@ class MarkupParser(PatchedHTMLParser):
|
|||
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 @@ class MarkupParser(PatchedHTMLParser):
|
|||
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:
|
||||
|
|
|
|||
|
|
@ -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 @@ class TestCSP(TestCase):
|
|||
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,7 +187,7 @@ 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)
|
||||
|
||||
|
||||
|
|
@ -194,7 +195,7 @@ 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,7 +204,7 @@ 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)
|
||||
|
|
@ -211,8 +212,7 @@ def test_proper_line_numbers():
|
|||
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():
|
||||
|
|
|
|||
|
|
@ -97,7 +97,7 @@ class TestWebapps(TestCase):
|
|||
"type": "web",
|
||||
}
|
||||
|
||||
self.resources = []
|
||||
self.resources = [("app_type", "web")]
|
||||
|
||||
def analyze(self):
|
||||
"""Run the webapp tests on the file."""
|
||||
|
|
|
|||
|
|
@ -1,3 +1,5 @@
|
|||
from nose.tools import eq_
|
||||
|
||||
from helper import TestCase
|
||||
from appvalidator.errorbundle import ErrorBundle
|
||||
import appvalidator.testcases.webappbase as appbase
|
||||
|
|
@ -24,6 +26,8 @@ class TestWebappPermissions(TestCase):
|
|||
def analyze(self):
|
||||
self.err.save_resource(
|
||||
"permissions", self.manifest.get("permissions", {}).keys())
|
||||
self.err.save_resource(
|
||||
"app_type", self.manifest.get("type", "web"))
|
||||
appbase.test_permissions(self.err, None)
|
||||
|
||||
def test_no_perms(self):
|
||||
|
|
@ -35,6 +39,7 @@ class TestWebappPermissions(TestCase):
|
|||
self.manifest["type"] = "certified"
|
||||
self.analyze()
|
||||
self.assert_silent()
|
||||
eq_(self.err.get_resource("app_type"), "certified")
|
||||
|
||||
def test_certified_perms_priv(self):
|
||||
self.manifest["permissions"][CERT_PERM] = True
|
||||
|
|
@ -58,6 +63,7 @@ class TestWebappPermissions(TestCase):
|
|||
self.manifest["type"] = "privileged"
|
||||
self.analyze()
|
||||
self.assert_silent()
|
||||
eq_(self.err.get_resource("app_type"), "privileged")
|
||||
|
||||
def test_privileged_perms_cert(self):
|
||||
self.manifest["permissions"][PRIV_PERM] = True
|
||||
|
|
@ -93,8 +99,10 @@ class TestWebappPermissions(TestCase):
|
|||
self.manifest["type"] = "web"
|
||||
self.analyze()
|
||||
self.assert_silent()
|
||||
eq_(self.err.get_resource("app_type"), "web")
|
||||
|
||||
def test_web_perms_web_implicit(self):
|
||||
self.manifest["permissions"][WEB_PERM] = True
|
||||
self.analyze()
|
||||
self.assert_silent()
|
||||
eq_(self.err.get_resource("app_type"), "web")
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче