Return values should be considered dynamic values

appvalidator/testcases/javascript/actions.py

@@ -395,7 +395,7 @@ def _call_expression(traverser, node):
     if member.is_global and "return" in member.value:
         return member.value["return"](wrapper=member, arguments=args,
                                       traverser=traverser)
-    return True
+    return JSWrapper(traverser=traverser)
 
 
 def _call_settimeout(a, t, e):

appvalidator/testcases/javascript/instanceproperties.py

@@ -5,6 +5,10 @@ from appvalidator.constants import BUGZILLA_BUG
 import jstypes
 
 
+EVENT_ASSIGNMENT = re.compile("<.+ on[a-z]+=")
+JS_URL = re.compile("href=[\'\"]javascript:")
+
+
 def set_innerHTML(new_value, traverser):
     """Tests that values being assigned to innerHTML are not dangerous."""
     return _set_HTML_property("innerHTML", new_value, traverser)
@@ -24,9 +28,8 @@ def _set_HTML_property(function, new_value, traverser):
         if isinstance(literal_value, types.StringTypes):
             # Static string assignments
 
-            # Test for on* attributes
-            event_assignment = re.compile("<.+ on[a-z]+=")
-            if event_assignment.search(literal_value.lower()):
+            # Test for on* attributes and script tags.
+            if EVENT_ASSIGNMENT.search(literal_value.lower()):
                 traverser.err.warning(
                     err_id=("testcases_javascript_instancetypes",
                             "set_%s" % function, "event_assignment"),
@@ -41,6 +44,21 @@ def _set_HTML_property(function, new_value, traverser):
                     line=traverser.line,
                     column=traverser.position,
                     context=traverser.context)
+            elif ("<script" in literal_value or
+                  JS_URL.search(literal_value)):
+                traverser.err.warning(
+                    err_id=("testcases_javascript_instancetypes",
+                            "set_%s" % function, "script_assignment"),
+                    warning="Scripts should not be created with `%s`" %
+                                function,
+                    description="`%s` should not be used to add scripts to "
+                                "pages via script tags or JavaScript URLs. "
+                                "Instead, use event listeners and external "
+                                "JavaScript." % function,
+                    filename=traverser.filename,
+                    line=traverser.line,
+                    column=traverser.position,
+                    context=traverser.context)
             else:
                 # Everything checks out, but we still want to pass it through
                 # the markup validator. Turn off strict mode so we don't get

jsmain.py

@@ -3,13 +3,14 @@
 import sys
 import os
 
-from validator.errorbundler import ErrorBundle
-from validator.outputhandlers.shellcolors import OutputHandler
-import validator.testcases.scripting as scripting
-import validator.testcases.javascript.traverser
-from validator.testcases.javascript.predefinedentities import GLOBAL_ENTITIES
-import validator.testcases.javascript.spidermonkey as spidermonkey
-validator.testcases.javascript.traverser.DEBUG = True
+from appvalidator.constants import SPIDERMONKEY_INSTALLATION
+from appvalidator.errorbundler import ErrorBundle
+from appvalidator.outputhandlers.shellcolors import OutputHandler
+import appvalidator.testcases.scripting as scripting
+import appvalidator.testcases.javascript.traverser
+from appvalidator.testcases.javascript.predefinedentities import GLOBAL_ENTITIES
+import appvalidator.testcases.javascript.spidermonkey as spidermonkey
+appvalidator.testcases.javascript.traverser.DEBUG = True
 
 if __name__ == '__main__':
     err = ErrorBundle(instant=True)
@@ -22,7 +23,7 @@ if __name__ == '__main__':
                                filename=path,
                                data=script)
     else:
-        trav = validator.testcases.javascript.traverser.Traverser(err, "stdin")
+        trav = appvalidator.testcases.javascript.traverser.Traverser(err, "stdin")
         trav._push_context()
 
         def do_inspect(wrapper, arguments, traverser):
@@ -50,8 +51,7 @@ if __name__ == '__main__':
         GLOBAL_ENTITIES[u"exit"] = {"return": do_exit}
 
         while True:
-            line = sys.stdin.readline()
-
+            line = raw_input("js> ")
             if line == "enable bootstrap\n":
                 err.save_resource("em:bootstrap", True)
                 continue
@@ -73,7 +73,7 @@ if __name__ == '__main__':
                     print actions[vars[0]](wrap)
                 continue
 
-            tree = spidermonkey.get_tree(line, err)
+            tree = spidermonkey.get_tree(line, err, shell=SPIDERMONKEY_INSTALLATION)
             if tree is None:
                 continue
             tree = tree["body"]

tests/js/test_instanceproperties.py

@@ -1,6 +1,12 @@
+from mock import patch
+
 from js_helper import _do_test_raw, TestCase
 
 
+def _mock_html_error(self, *args, **kwargs):
+    self.err.error(("foo", "bar"), "Does not pass validation.")
+
+
 class TestHTML(TestCase):
 
     def test_innerHTML(self):
@@ -19,6 +25,8 @@ class TestHTML(TestCase):
             yield test, self, decl, '"<div></div>"', False
             yield test, self, decl, '"<div onclick=\\"foo\\"></div>"', True
             yield test, self, decl, '"x" + y', True
+            yield test, self, decl, '<a href="javascript:alert();">', True
+            yield test, self, decl, '"<script>"', True
 
     def test_outerHTML(self):
         """Test that the dev can't define event handler in outerHTML."""
@@ -37,6 +45,8 @@ class TestHTML(TestCase):
             yield test, self, decl, '"<div onclick=\\"foo\\"></div>"', True
             yield test, self, decl, '"x" + y', True
 
+    @patch("appvalidator.testcases.markup.markuptester.MarkupParser.process",
+           _mock_html_error)
     def test_complex_innerHTML(self):
         """
         Tests that innerHTML can't be assigned an HTML chunk with bad code.
@@ -44,10 +54,20 @@ class TestHTML(TestCase):
 
         self.run_script("""
         var x = foo();
-        x.innerHTML = "<script src=\\"http://foo.bar/\\"></script>";
+        x.innerHTML = "<b></b>";
         """)
         self.assert_failed(with_errors=True)
 
+    def test_function_return(self):
+        """
+        Test that the return value of a function is considered a dynamic value.
+        """
+
+        self.run_script("""
+        x.innerHTML = foo();
+        """)
+        self.assert_failed()
+
 
 class TestOnProperties(TestCase):