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

Merge Develop into Release
semgrep · Apr 25, 2024 · 0279a95 · 0279a95
2 parents 8a79505 + 2c0ea0a
commit 0279a95
Show file tree

Hide file tree

Showing 4 changed files with 231 additions and 11 deletions.
diff --git a/php/lang/security/injection/echoed-request.php b/php/lang/security/injection/echoed-request.php
@@ -1,18 +1,18 @@
 <?php
 
 // example key-value: name=%3Cscript%3Econfirm%28%29%3C%2Fscript%3E
-function dangerousPrintUsage() {
+function dangerousEchoUsage() {
     $name = $_REQUEST['name'];
     // ruleid: echoed-request
-    print("Hello : $name");
+    echo "Hello : $name";
     // ruleid: echoed-request
-    print("Hello : " . $name);
+    echo "Hello : " . $name;
 }
 
-function safePrintUsage() {
+function safeEchoUsage() {
     $name = $_REQUEST['name'];
     // ok: echoed-request
-    print("Hello : " . htmlentities($name));
+    echo "Hello : " . htmlentities($name);
 }
 
 function doSmth() {
@@ -44,6 +44,22 @@ function doSmth5() {
     echo "Hello ".trim($_POST['name']);
 }
 
+function doSmth6() {
+   $VAR = $_GET['someval'];
+    if(isset($VAR)){ 
+        // ruleid: echoed-request
+        echo $VAR; 
+    }
+}
+
+function doSmth7() {
+    $VAR = $_GET['someval'];
+     if(empty($VAR)){ 
+         // ruleid: echoed-request
+         echo $VAR; 
+     }
+ }
+
 function doOK1() {
     // ok: echoed-request
     echo "Hello ".htmlentities($_POST['name'])." !";
@@ -63,11 +79,11 @@ function doOK3() {
 
 function doOK4() {
     // ok: echoed-request
-    echo "Hello ".isset($_POST['name'])." !";
+    echo "Hello ".e($_POST['name'])." !";
 }
 
 function doOK5() {
-    $safevar = empty($_GET['name']);
+    $safevar = esc_attr($_GET['name']);
     // ok: echoed-request
     echo "Hello $safevar !";
 }
@@ -84,4 +100,14 @@ function doOK7() {
     echo $safevar;
 }
 
+function doOK8() {
+    // ok: echoed-request
+    echo "Hello ".isset($_POST['name'])." !";
+}
+
+function doOK9() {
+    $safevar = empty($_GET['name']);
+    // ok: echoed-request
+    echo "Hello $safevar !";
+}
 
diff --git a/php/lang/security/injection/echoed-request.yaml b/php/lang/security/injection/echoed-request.yaml
@@ -11,13 +11,33 @@ rules:
   - pattern: $_GET
   - pattern: $_POST
   pattern-sinks:
-  - pattern: echo ...;
-  - pattern: print(...);
+  - pattern: echo $...VARS;
   pattern-sanitizers:
-  - pattern: isset(...)
-  - pattern: empty(...)
   - pattern: htmlentities(...)
   - pattern: htmlspecialchars(...)
+  - pattern: strip_tags(...)
+  - pattern: isset(...)
+  - pattern: empty(...)
+  # Wordpress Escapes
+  - pattern: esc_html(...)
+  - pattern: esc_attr(...)
+  - pattern: wp_kses(...)
+  # Laravel Escapes
+  - pattern: e(...)
+  # Symfony Escapes
+  - pattern: twig_escape_filter(...)
+  # CodeIgniter Escapes
+  - pattern: xss_clean(...)
+  - pattern: html_escape(...)
+  # Drupal Escapes
+  - pattern: Html::escape(...)
+  - pattern: Xss::filter(...)
+  # Magento Escapes
+  - pattern: escapeHtml(...)
+  # Laminas Escapes
+  - pattern: escapeHtml(...)
+  - pattern: escapeHtmlAttr(...)
+  fix: echo htmlentities($...VARS);
   metadata:
     technology:
     - php

diff --git a/php/lang/security/injection/printed-request.php b/php/lang/security/injection/printed-request.php
@@ -0,0 +1,112 @@
+<?php
+
+// example key-value: name=%3Cscript%3Econfirm%28%29%3C%2Fscript%3E
+function dangerousEchoUsage() {
+    $name = $_REQUEST['name'];
+    // ruleid: printed-request
+    print("Hello : $name");
+    // ruleid: printed-request
+    print("Hello : " . $name);
+}
+
+function safeEchoUsage() {
+    $name = $_REQUEST['name'];
+    // ok: printed-request
+    print("Hello : " . htmlentities($name));
+}
+
+function doSmth() {
+    $name = $_REQUEST['name'];
+    // ruleid: printed-request
+    print("Hello :".$name);
+}
+
+function doSmth2() {
+    // ruleid: printed-request
+    print("Hello ".$_POST['name']." !");
+}
+
+function doSmth3() {
+    $name = $_GET['name'];
+    if (str_contains($name, 'foobar')) {
+        // ruleid: printed-request
+        print("Hello :".$name);
+    }
+}
+
+function doSmth4() {
+    // ruleid: printed-request
+    print("Hello ".htmlentities($_POST['name'])." !".$_POST['lastname']);
+}
+
+function doSmth5() {
+     // ruleid: printed-request
+    print("Hello ".trim($_POST['name']));
+}
+
+function doSmth6() {
+    $VAR = $_GET['someval'];
+     if(isset($VAR)){ 
+         // ruleid: printed-request
+         print($VAR); 
+     }
+ }
+
+ function doSmth7() {
+     $VAR = $_GET['someval'];
+      if(empty($VAR)){ 
+          // ruleid: printed-request
+          print($VAR); 
+      }
+  }
+
+function doOK1() {
+    // ok: printed-request
+    print("Hello ".htmlentities($_POST['name'])." !");
+}
+
+function doOK2() {
+    $input = $_GET['name'];
+    // ok: printed-request
+    print("Hello ".htmlspecialchars($input)." !");
+}
+
+function doOK3() {
+    $safevar = "Hello ".htmlentities(trim($_GET['name']));
+    // ok: printed-request
+    print($safevar);
+}
+
+function doOK4() {
+    // ok: printed-request
+    print("Hello ".e($_POST['name'])." !");
+}
+
+function doOK5() {
+    $safevar = esc_attr($_GET['name']);
+    // ok: printed-request
+    print("Hello $safevar !");
+}
+
+function doOK6() {
+    $safevar = "Hello ".htmlentities($_GET['name']);
+    // ok: printed-request
+    print($safevar);
+}
+
+function doOK7() {
+    $safevar = "Hello ".htmlspecialchars($_GET['name']);
+    // ok: printed-request
+    print($safevar);
+}
+
+function doOK8() {
+    // ok: printed-request
+    print("Hello ".isset($_POST['name'])." !");
+}
+
+function doOK9() {
+    $safevar = empty($_GET['name']);
+    // ok: printed-request
+    print("Hello $safevar !");
+}
diff --git a/php/lang/security/injection/printed-request.yaml b/php/lang/security/injection/printed-request.yaml
@@ -0,0 +1,62 @@
+rules:
+- id: printed-request
+  mode: taint
+  message: >-
+    `Printing user input risks cross-site scripting vulnerability.
+    You should use `htmlentities()` when showing data to users.
+  languages: [php]
+  severity: ERROR
+  pattern-sources:
+  - pattern: $_REQUEST
+  - pattern: $_GET
+  - pattern: $_POST
+  pattern-sinks:
+  - pattern: print($...VARS);
+  pattern-sanitizers:
+  - pattern: htmlentities(...)
+  - pattern: htmlspecialchars(...)
+  - pattern: strip_tags(...)
+  - pattern: isset(...)
+  - pattern: empty(...)
+  # Wordpress Escapes
+  - pattern: esc_html(...)
+  - pattern: esc_attr(...)
+  - pattern: wp_kses(...)
+  # Laravel Escapes
+  - pattern: e(...)
+  # Symfony Escapes
+  - pattern: twig_escape_filter(...)
+  # CodeIgniter Escapes
+  - pattern: xss_clean(...)
+  - pattern: html_escape(...)
+  # Drupal Escapes
+  - pattern: Html::escape(...)
+  - pattern: Xss::filter(...)
+  # Magento Escapes
+  - pattern: escapeHtml(...)
+  # Laminas Escapes
+  - pattern: escapeHtml(...)
+  - pattern: escapeHtmlAttr(...)
+  fix: print(htmlentities($...VARS));
+  metadata:
+    technology:
+    - php
+    cwe:
+    - "CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')"
+    owasp:
+    - A07:2017 - Cross-Site Scripting (XSS)
+    - A03:2021 - Injection
+    category: security
+    references:
+    - https://www.php.net/manual/en/function.htmlentities.php
+    - https://www.php.net/manual/en/reserved.variables.request.php
+    - https://www.php.net/manual/en/reserved.variables.post.php
+    - https://www.php.net/manual/en/reserved.variables.get.php
+    - https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html
+    cwe2022-top25: true
+    cwe2021-top25: true
+    subcategory:
+    - vuln
+    likelihood: MEDIUM
+    impact: MEDIUM
+    confidence: MEDIUM