Allow string literals assigned to attributes in BanCreateDom.

Similar to BANNED_PROPERTY_NON_CONSTANT_WRITE, string literals are not dangerous as they couldn't be attacker controlled. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=150198867
google · Mar 16, 2017 · 5fb6288 · 5fb6288
1 parent b2f2d2f
commit 5fb6288
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 7 deletions.
diff --git a/src/com/google/javascript/jscomp/ConformanceRules.java b/src/com/google/javascript/jscomp/ConformanceRules.java
@@ -1428,6 +1428,8 @@ protected ConformanceResult checkConformance(NodeTraversal t, Node n) {
   /**
    * Ban {@code goog.dom.createDom} and {@code goog.dom.DomHelper#createDom} with parameters
    * specified in {@code value} in the format tagname.attribute, e.g. {@code value: 'iframe.src'}.
+   * Note that string literal values assigned to banned attributes are allowed as they couldn't be
+   * attacker controlled.
    */
   public static final class BanCreateDom extends AbstractRule {
     private List<String[]> bannedTagAttrs;
@@ -1469,7 +1471,12 @@ protected ConformanceResult checkConformance(NodeTraversal t, Node n) {
           // Attrs is not an object literal and tagName matches or is unknown.
           return ConformanceResult.POSSIBLE_VIOLATION;
         }
-        if (NodeUtil.getFirstPropMatchingKey(attrs, tagAttr[1]) != null) {
+        Node prop = NodeUtil.getFirstPropMatchingKey(attrs, tagAttr[1]);
+        if (prop != null) {
+          if (NodeUtil.isStringLiteralValue(prop)) {
+            // Ignore string literal values.
+            continue;
+          }
           return tagName == null
               ? ConformanceResult.POSSIBLE_VIOLATION
               : ConformanceResult.VIOLATION;

diff --git a/test/com/google/javascript/jscomp/CheckConformanceTest.java b/test/com/google/javascript/jscomp/CheckConformanceTest.java
@@ -1682,24 +1682,24 @@ public void testBanCreateDom() {
         "}";
 
     testWarning(
-        "goog.dom.createDom('iframe', {'src': ''});",
+        "goog.dom.createDom('iframe', {'src': src});",
         CheckConformance.CONFORMANCE_VIOLATION,
         "Violation: BanCreateDom Message");
 
     testWarning(
-        "goog.dom.createDom('iframe', {'src': '', 'name': ''}, '');",
+        "goog.dom.createDom('iframe', {'src': src, 'name': ''}, '');",
         CheckConformance.CONFORMANCE_VIOLATION,
         "Violation: BanCreateDom Message");
 
     testWarning(
-        "goog.dom.createDom(goog.dom.TagName.IFRAME, {'src': ''});",
+        "goog.dom.createDom(goog.dom.TagName.IFRAME, {'src': src});",
         CheckConformance.CONFORMANCE_VIOLATION,
         "Violation: BanCreateDom Message");
 
     // TODO(jakubvrana): Add a test for goog.dom.DomHelper.
 
     testWarning(
-        "goog.dom.createDom(tag, {'src': ''});",
+        "goog.dom.createDom(tag, {'src': src});",
         CheckConformance.CONFORMANCE_POSSIBLE_VIOLATION,
         "Possible violation: BanCreateDom Message");
 
@@ -1714,8 +1714,9 @@ public void testBanCreateDom() {
         "Possible violation: BanCreateDom Message");
 
     testSame("goog.dom.createDom('iframe');");
-    testSame("goog.dom.createDom('iframe', {'name': ''});");
-    testSame("goog.dom.createDom('img', {'src': ''});");
+    testSame("goog.dom.createDom('iframe', {'src': ''});");
+    testSame("goog.dom.createDom('iframe', {'name': name});");
+    testSame("goog.dom.createDom('img', {'src': src});");
     testSame("goog.dom.createDom('img', attrs);");
     testSame("goog.dom.createDom(tag, {});");
   }