CWE-830 add support for setting attributes via setAttribute method

Author: Stephan Brandauer

javascript/ql/src/Security/CWE-830/FunctionalityFromUntrustedSource.ql

@@ -90,13 +90,22 @@ module DynamicCreation {
     call.getArgument(0).getStringValue().toLowerCase() = name
   }
 
+  DataFlow::Node getAttributeAssignmentRhs(DataFlow::CallNode createCall, string name) {
+    result = createCall.getAPropertyWrite(name).getRhs()
+    or
+    exists(DataFlow::InvokeNode inv | inv = createCall.getAMemberInvocation("setAttribute") |
+      inv.getArgument(0).getStringValue() = name and
+      result = inv.getArgument(1)
+    )
+  }
+
   /**
    * Holds if `createCall` creates a `<script ../>` element which never
    * has its `integrity` attribute set locally.
    */
   predicate isCreateScriptNodeWoIntegrityCheck(DataFlow::CallNode createCall) {
     isCreateElementNode(createCall, "script") and
-    not exists(createCall.getAPropertyWrite("integrity"))
+    not exists(getAttributeAssignmentRhs(createCall, "integrity"))
   }
 
   DataFlow::Node urlTrackedFromUnsafeSourceLiteral(DataFlow::TypeTracker t) {
@@ -126,15 +135,17 @@ module DynamicCreation {
     result = urlTrackedFromUnsafeSourceLiteral(DataFlow::TypeTracker::end())
   }
 
+  /** Holds if `sink` is assigned to the attribute `name` of any HTML element. */
   predicate isAssignedToSrcAttribute(string name, DataFlow::Node sink) {
     exists(DataFlow::CallNode createElementCall |
-      name = "script" and
-      isCreateScriptNodeWoIntegrityCheck(createElementCall) and
-      sink = createElementCall.getAPropertyWrite("src").getRhs()
-      or
-      name = "iframe" and
-      isCreateElementNode(createElementCall, "iframe") and
-      sink = createElementCall.getAPropertyWrite("src").getRhs()
+      sink = getAttributeAssignmentRhs(createElementCall, "src") and
+      (
+        name = "script" and
+        isCreateScriptNodeWoIntegrityCheck(createElementCall)
+        or
+        name = "iframe" and
+        isCreateElementNode(createElementCall, "iframe")
+      )
     )
   }
 
@@ -143,8 +154,8 @@ module DynamicCreation {
 
     IframeOrScriptSrcAssignment() {
       exists(DataFlow::Node n | n.asExpr() = this |
-        DynamicCreation::isAssignedToSrcAttribute(name, n) and
-        n = DynamicCreation::urlTrackedFromUnsafeSourceLiteral()
+        isAssignedToSrcAttribute(name, n) and
+        n = urlTrackedFromUnsafeSourceLiteral()
       )
     }
 

javascript/ql/test/query-tests/Security/CWE-830/DynamicCreationOfUntrustedSourceUse.html

@@ -33,6 +33,13 @@
               var ifrm3 = document.createElement('iframe');
               ifrm3.src = getUrl('v123');
 
+              // NOT OK (assignment of bad URL using setAttribute)
+              var ifrm4 = document.createElement('iframe');
+              ifrm4.setAttribute('src', 'http://www.example.local/page.html');
+
+              // OK (bad URL, but the attribute is not `src`)
+              var ifrm5 = document.createElement('iframe');
+              ifrm5.setAttribute('data-src', 'http://www.example.local/page.html');
             })();
           </script>
     </head>

javascript/ql/test/query-tests/Security/CWE-830/FunctionalityFromUntrustedSource.expected

@@ -1,6 +1,7 @@
 | DynamicCreationOfUntrustedSourceUse.html:19:28:19:129 | ('https ... /ga.js' | HTML script element loaded using unencrypted connection. |
 | DynamicCreationOfUntrustedSourceUse.html:23:26:23:50 | 'http:/ ... e.com/' | HTML iframe element loaded using unencrypted connection. |
 | DynamicCreationOfUntrustedSourceUse.html:34:27:34:40 | getUrl('v123') | HTML iframe element loaded using unencrypted connection. |
+| DynamicCreationOfUntrustedSourceUse.html:38:41:38:76 | 'http:/ ... e.html' | HTML iframe element loaded using unencrypted connection. |
 | StaticCreationOfUntrustedSourceUse.html:6:9:6:56 | <script>...</> | HTML script element loaded using unencrypted connection. |
 | StaticCreationOfUntrustedSourceUse.html:9:9:9:58 | <iframe>...</> | HTML iframe element loaded using unencrypted connection. |
 | StaticCreationOfUntrustedSourceUse.html:21:9:21:155 | <script>...</> | Script loaded from content delivery network with no integrity check. |