Merge pull request #3290 from semgrep/merge-develop-to-release

Merge Develop into Release

csharp/lang/security/injections/os-command.cs

@@ -116,5 +116,65 @@ public void RunConstantAppWithArgs(string args)
             // ok: os-command-injection
             var process = Process.Start(processStartInfo);
         }
+
+        public void RunOsCommandAndArgsWithProcessParam(string command, string arguments)
+        {
+            Process process = new Process
+            {
+                StartInfo = new ProcessStartInfo
+                {
+                    FileName = command,
+                    Arguments = args
+                }
+            };
+
+            // ruleid: os-command-injection
+            process.Start();
+        }
+
+        public void RunOsCommandAndArgsWithProcessParam(string command, string arguments)
+        {
+            Process process = new Process
+            {
+                StartInfo = new ProcessStartInfo
+                {
+                    FileName = "constant",
+                    Arguments = arguments
+                }
+            };
+
+            // ruleid: os-command-injection
+            process.Start();
+        }
+
+        public void RunOsCommandAndArgsWithProcessParam(string command, string arguments)
+        {
+            Process process = new Process
+            {
+                StartInfo = new ProcessStartInfo
+                {
+                    FileName = command,
+                    Arguments = "constant"
+                }
+            };
+
+            // ruleid: os-command-injection
+            process.Start();
+        }
+
+        public void RunOsCommandAndArgsWithProcessParam(string command, string arguments)
+        {
+            Process process = new Process
+            {
+                StartInfo = new ProcessStartInfo
+                {
+                    FileName = "constant",
+                    Arguments = "constant"
+                }
+            };
+
+            // ok: os-command-injection
+            process.Start();
+        }
     }
 }

csharp/lang/security/injections/os-command.yaml

@@ -72,3 +72,22 @@ rules:
       - pattern: |
           Process.Start($PSINFO);
       - focus-metavariable: $PSINFO
+    - patterns:
+        - pattern-inside: |
+            Process $PROC = new Process()
+            {
+              StartInfo = new ProcessStartInfo()
+              {
+                ...
+              }
+            };
+            ...
+        - pattern-either:
+            - pattern-inside: |
+                FileName = $ARG;
+                ...
+            - pattern-inside: |
+                Arguments = $ARG;
+                ...
+        - pattern: |
+            $PROC.Start();

java/lang/security/audit/xxe/documentbuilderfactory-disallow-doctype-decl-missing.fixed.java

@@ -42,15 +42,15 @@ public void BadDocumentBuilderFactory() throws  ParserConfigurationException {
         DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
         //ruleid:documentbuilderfactory-disallow-doctype-decl-missing
         dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
-dbf.newDocumentBuilder();
+        dbf.newDocumentBuilder();
     }
 
     public void BadDocumentBuilderFactory2() throws  ParserConfigurationException {
         DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
         dbf.setFeature("somethingElse", true);
         //ruleid:documentbuilderfactory-disallow-doctype-decl-missing
         dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
-dbf.newDocumentBuilder();
+        dbf.newDocumentBuilder();
     }
 }
 
@@ -77,7 +77,7 @@ class BadDocumentBuilderFactoryStatic {
     public void doSomething(){
         //ruleid:documentbuilderfactory-disallow-doctype-decl-missing
         dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
-dbf.newDocumentBuilder();
+        dbf.newDocumentBuilder();
     }
 
 }
@@ -115,7 +115,7 @@ public void GoodDocumentBuilderFactory(boolean condition) throws  ParserConfigur
         }
         //ruleid:documentbuilderfactory-disallow-doctype-decl-missing
         dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
-dbf.newDocumentBuilder();
+        dbf.newDocumentBuilder();
     }
 
     private DocumentBuilderFactory newFactory(){

python/django/security/audit/unvalidated-password.yaml

@@ -27,7 +27,7 @@ rules:
   - pattern: $MODEL.set_password($X)
   fix: >
     if django.contrib.auth.password_validation.validate_password($X, user=$MODEL):
-            $MODEL.set_password($X)
+        $MODEL.set_password($X)
   message: >-
     The password on '$MODEL' is being set without validating the password.
     Call django.contrib.auth.password_validation.validate_password() with

python/lang/correctness/list-modify-iterating.py

@@ -30,3 +30,8 @@
 for i in e:
     print(i)
     d.append(i)
+
+# ruleid:list-modify-while-iterate
+for i in e:
+    if i == 1:
+        e.remove(i)

python/lang/correctness/list-modify-iterating.yaml

@@ -22,7 +22,13 @@ rules:
           for $ELEMENT in $LIST:
             ...
             $LIST.extend(...)
+      - pattern: |
+          for $ELEMENT in $LIST:
+            ...
+            $LIST.remove(...)
     metadata:
       category: correctness
       technology:
         - python
+      references:
+        - https://unspecified.wordpress.com/2009/02/12/thou-shalt-not-modify-a-list-during-iteration/

yaml/kubernetes/security/allow-privilege-escalation-no-securitycontext.fixed.test.yaml

@@ -3,9 +3,9 @@ kind: Pod
 spec:
   containers:
     # ruleid: allow-privilege-escalation-no-securitycontext
-    - name: nginx
-      securityContext:
+    - securityContext:
         allowPrivilegeEscalation: false
+      name: nginx
       image: nginx
     # ok: allow-privilege-escalation-no-securitycontext
     - name: postgres

yaml/kubernetes/security/allow-privilege-escalation-no-securitycontext.yaml

@@ -5,7 +5,7 @@ rules:
       containers:
         ...
   - pattern-inside: |
-      - name: $CONTAINER
+      - $NAME: $CONTAINER
         ...
   - pattern: |
       image: ...
@@ -15,22 +15,25 @@ rules:
       ...
       securityContext:
         ...
-  - focus-metavariable: $CONTAINER
+  - metavariable-regex:
+      metavariable: $NAME
+      regex: "name"
+  - focus-metavariable: $NAME
   fix: |
-    $CONTAINER
-          securityContext:
-            allowPrivilegeEscalation: false
+    securityContext:
+      allowPrivilegeEscalation: false
+    $NAME
   message: >-
-    In Kubernetes, each pod runs in its own isolated environment with its own 
-    set of security policies. However, certain container images may contain 
-    `setuid` or `setgid` binaries that could allow an attacker to perform 
-    privilege escalation and gain access to sensitive resources. To mitigate 
-    this risk, it's recommended to add a `securityContext` to the container in 
-    the pod, with the parameter `allowPrivilegeEscalation` set to `false`. 
-    This will prevent the container from running any privileged processes and 
-    limit the impact of any potential attacks. 
-    By adding a `securityContext` to your Kubernetes pod, you can help to 
-    ensure that your containerized applications are more secure and less 
+    In Kubernetes, each pod runs in its own isolated environment with its own
+    set of security policies. However, certain container images may contain
+    `setuid` or `setgid` binaries that could allow an attacker to perform
+    privilege escalation and gain access to sensitive resources. To mitigate
+    this risk, it's recommended to add a `securityContext` to the container in
+    the pod, with the parameter `allowPrivilegeEscalation` set to `false`.
+    This will prevent the container from running any privileged processes and
+    limit the impact of any potential attacks.
+    By adding a `securityContext` to your Kubernetes pod, you can help to
+    ensure that your containerized applications are more secure and less
     vulnerable to privilege escalation attacks.
   metadata:
     cwe:

yaml/kubernetes/security/allow-privilege-escalation.yaml

@@ -27,19 +27,19 @@ rules:
   - focus-metavariable: $SC
   fix: |
     securityContext:
-            allowPrivilegeEscalation: false #
+      allowPrivilegeEscalation: false #
   message: >-
-    In Kubernetes, each pod runs in its own isolated environment with its own 
-    set of security policies. However, certain container images may contain 
-    `setuid` or `setgid` binaries that could allow an attacker to perform 
-    privilege escalation and gain access to sensitive resources. To mitigate 
-    this risk, it's recommended to add a `securityContext` to the container in 
-    the pod, with the parameter `allowPrivilegeEscalation` set to `false`. 
-    This will prevent the container from running any privileged processes and 
-    limit the impact of any potential attacks. 
-    By adding the `allowPrivilegeEscalation` parameter to your the 
-    `securityContext`, you can help to 
-    ensure that your containerized applications are more secure and less 
+    In Kubernetes, each pod runs in its own isolated environment with its own
+    set of security policies. However, certain container images may contain
+    `setuid` or `setgid` binaries that could allow an attacker to perform
+    privilege escalation and gain access to sensitive resources. To mitigate
+    this risk, it's recommended to add a `securityContext` to the container in
+    the pod, with the parameter `allowPrivilegeEscalation` set to `false`.
+    This will prevent the container from running any privileged processes and
+    limit the impact of any potential attacks.
+    By adding the `allowPrivilegeEscalation` parameter to your the
+    `securityContext`, you can help to
+    ensure that your containerized applications are more secure and less
     vulnerable to privilege escalation attacks.
   metadata:
     cwe:

yaml/kubernetes/security/run-as-non-root-container-level-missing-security-context.fixed.test.yaml

@@ -22,9 +22,9 @@ spec:
   containers:
     - name: nginx
     # ruleid: run-as-non-root-container-level-missing-security-context
-      image: nginx
       securityContext:
         runAsNonRoot: true
+      image: nginx
     - name: postgres
       image: postgres
       # this is okay because there already is a security context, requires different fix, different rule

yaml/kubernetes/security/run-as-non-root-container-level-missing-security-context.yaml

@@ -37,31 +37,34 @@ rules:
   # Capture container image
   - pattern: |
           - name: $CONTAINER
-            image: $IMAGE
+            $IMAGE: $IMAGEVAL
             ...
   # But missing securityContext
   - pattern-not: |
           - name: $CONTAINER
-            image: $IMAGE
+            image: $IMAGEVAL
             ...
             securityContext:
               ...
+  - metavariable-regex:
+      metavariable: $IMAGE
+      regex: "image"
   - focus-metavariable: $IMAGE
   fix: |
+    securityContext:
+      runAsNonRoot: true
     $IMAGE
-          securityContext:
-            runAsNonRoot: true
   message: >-
-    When running containers in Kubernetes, it's important to ensure that they 
-    are properly secured to prevent privilege escalation attacks. 
-    One potential vulnerability is when a container is allowed to run 
-    applications as the root user, which could allow an attacker to gain 
-    access to sensitive resources. To mitigate this risk, it's recommended to 
-    add a `securityContext` to the container, with the parameter `runAsNonRoot` 
-    set to `true`. This will ensure that the container runs as a non-root user, 
-    limiting the damage that could be caused by any potential attacks. By 
-    adding a `securityContext` to the container in your Kubernetes pod, you can 
-    help to ensure that your containerized applications are more secure and 
+    When running containers in Kubernetes, it's important to ensure that they
+    are properly secured to prevent privilege escalation attacks.
+    One potential vulnerability is when a container is allowed to run
+    applications as the root user, which could allow an attacker to gain
+    access to sensitive resources. To mitigate this risk, it's recommended to
+    add a `securityContext` to the container, with the parameter `runAsNonRoot`
+    set to `true`. This will ensure that the container runs as a non-root user,
+    limiting the damage that could be caused by any potential attacks. By
+    adding a `securityContext` to the container in your Kubernetes pod, you can
+    help to ensure that your containerized applications are more secure and
     less vulnerable to privilege escalation attacks.
   metadata:
     references:

yaml/kubernetes/security/run-as-non-root-container-level.yaml

@@ -55,18 +55,18 @@ rules:
   - focus-metavariable: $SC
   fix: |
     $SC:
-            runAsNonRoot: true #
+      runAsNonRoot: true #
   message: >-
-    When running containers in Kubernetes, it's important to ensure that they 
-    are properly secured to prevent privilege escalation attacks. 
-    One potential vulnerability is when a container is allowed to run 
-    applications as the root user, which could allow an attacker to gain 
-    access to sensitive resources. To mitigate this risk, it's recommended to 
-    add a `securityContext` to the container, with the parameter `runAsNonRoot` 
-    set to `true`. This will ensure that the container runs as a non-root user, 
-    limiting the damage that could be caused by any potential attacks. By 
-    adding a `securityContext` to the container in your Kubernetes pod, you can 
-    help to ensure that your containerized applications are more secure and 
+    When running containers in Kubernetes, it's important to ensure that they
+    are properly secured to prevent privilege escalation attacks.
+    One potential vulnerability is when a container is allowed to run
+    applications as the root user, which could allow an attacker to gain
+    access to sensitive resources. To mitigate this risk, it's recommended to
+    add a `securityContext` to the container, with the parameter `runAsNonRoot`
+    set to `true`. This will ensure that the container runs as a non-root user,
+    limiting the damage that could be caused by any potential attacks. By
+    adding a `securityContext` to the container in your Kubernetes pod, you can
+    help to ensure that your containerized applications are more secure and
     less vulnerable to privilege escalation attacks.
   metadata:
     references: