From 9183fb0d808ca243a32634fbbc3a206342e5487d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alvaro=20Mu=C3=B1oz?= <pwntester@github.com>
Date: Sat, 20 Apr 2024 23:31:08 +0200
Subject: [PATCH] Fix expression always true query

---
 ql/lib/qlpack.yml                             |  2 +-
 .../CWE-571/ExpressionIsAlwaysTrue.ql         | 10 +++---
 ql/src/qlpack.yml                             |  2 +-
 .../CWE-571/.github/workflows/test.yml        | 35 +++++++++++++++++--
 .../CWE-571/ExpressionIsAlwaysTrue.expected   |  7 ++++
 5 files changed, 47 insertions(+), 9 deletions(-)
 create mode 100644 ql/test/query-tests/Security/CWE-571/ExpressionIsAlwaysTrue.expected

diff --git a/ql/lib/qlpack.yml b/ql/lib/qlpack.yml
index 4364e979f91..b557a60a751 100644
--- a/ql/lib/qlpack.yml
+++ b/ql/lib/qlpack.yml
@@ -2,7 +2,7 @@
 library: true
 warnOnImplicitThis: true
 name: githubsecuritylab/actions-all
-version: 0.0.13
+version: 0.0.14
 dependencies:
   codeql/util: ^0.2.0
   codeql/yaml: ^0.1.2
diff --git a/ql/src/Security/CWE-571/ExpressionIsAlwaysTrue.ql b/ql/src/Security/CWE-571/ExpressionIsAlwaysTrue.ql
index b631b5f17b3..58eab4c6022 100644
--- a/ql/src/Security/CWE-571/ExpressionIsAlwaysTrue.ql
+++ b/ql/src/Security/CWE-571/ExpressionIsAlwaysTrue.ql
@@ -1,4 +1,6 @@
 /**
+ *:
+ *
  * @name If expression always true
  * @description Expressions used in If conditions with extra spaces are always true.
  * @kind problem
@@ -16,10 +18,10 @@ import actions
 from If i
 where
   i.getCondition().matches("%${{%") and
-  i.getConditionStyle() = ["|", ">"]
-  or
-  i.getCondition().matches("%${{%") and
-  not i.getCondition().matches("${{%")
+  (
+    not i.getCondition().matches("${{%") or
+    not i.getCondition().matches("%}}")
+  )
   or
   count(i.getCondition().splitAt("${{")) > 2
 select i, "Expression always evaluates to true"
diff --git a/ql/src/qlpack.yml b/ql/src/qlpack.yml
index 6259340a4a6..99ecbe14d55 100644
--- a/ql/src/qlpack.yml
+++ b/ql/src/qlpack.yml
@@ -1,7 +1,7 @@
 ---
 library: false
 name: githubsecuritylab/actions-queries
-version: 0.0.13
+version: 0.0.14
 groups:
   - actions
   - queries
diff --git a/ql/test/query-tests/Security/CWE-571/.github/workflows/test.yml b/ql/test/query-tests/Security/CWE-571/.github/workflows/test.yml
index 16b725b5ee8..30c4dcab932 100644
--- a/ql/test/query-tests/Security/CWE-571/.github/workflows/test.yml
+++ b/ql/test/query-tests/Security/CWE-571/.github/workflows/test.yml
@@ -8,7 +8,7 @@ jobs:
   process-pr:
     runs-on: ubuntu-latest
     steps:
-      - name: Test1 
+      - name: Test 1 
         if: 1 == 2
         run: echo "Test 1 should not be printed"
       - name: Test 2 
@@ -36,8 +36,8 @@ jobs:
       - name: Test 7
         run: echo "Test 7 should not be printed"
         if: ${{ 
-              1 == 2 ||
-              3 == 4
+              github.actor == 'torvalds' || 
+              github.actor == 'dependabot[bot]'
           }}
 
       - name: Test 8
@@ -58,3 +58,32 @@ jobs:
       - name: Test 12
         if:   " ${{ 1 == 2  }}"
         run: echo "Test 12 should not be printed"
+      - name: Test 13
+        if: |
+          1 == 2 ||
+          3 == 4
+        run: echo "Test 13 should not be printed"
+      - name: Test 14
+        if: >-
+          ${{(
+            false || 1 == 2
+          )}}
+        run: echo "Test 14 should not be printed"
+      - name: Test 15
+        if: |-
+          ${{(
+            false || 1 == 2
+          )}}
+        run: echo "Test 15 should not be printed"
+      - name: Test 16
+        if: |+
+          ${{(
+            false || 1 == 2
+          )}}
+        run: echo "Test 16 should not be printed"
+      - name: Test 17
+        if: >+
+          ${{(
+            false || 1 == 2
+          )}}
+        run: echo "Test 17 should not be printed"
diff --git a/ql/test/query-tests/Security/CWE-571/ExpressionIsAlwaysTrue.expected b/ql/test/query-tests/Security/CWE-571/ExpressionIsAlwaysTrue.expected
new file mode 100644
index 00000000000..a8f068c9cd8
--- /dev/null
+++ b/ql/test/query-tests/Security/CWE-571/ExpressionIsAlwaysTrue.expected
@@ -0,0 +1,7 @@
+| .github/workflows/test.yml:15:13:19:13 | \| | Expression always evaluates to true |
+| .github/workflows/test.yml:34:13:34:39 | ${{ 1 = ... == 2 }} | Expression always evaluates to true |
+| .github/workflows/test.yml:45:13:48:24 | >  | Expression always evaluates to true |
+| .github/workflows/test.yml:56:15:56:31 | " ${{ 1 == 2  }}" | Expression always evaluates to true |
+| .github/workflows/test.yml:59:15:59:31 | " ${{ 1 == 2  }}" | Expression always evaluates to true |
+| .github/workflows/test.yml:79:13:82:14 | \|+ | Expression always evaluates to true |
+| .github/workflows/test.yml:85:13:88:14 | >+ | Expression always evaluates to true |