CVE-2023-52137: Improper Neutralization of Special Elements used in a Command ('Command Injection') (CWE-77)
value: The [`tj-actions/verify-changed-files`](https://github.com/tj-actions/verify-changed-files) action allows for command injection in changed filenames, allowing an attacker to execute arbitrary code and potentially leak secrets. The [`verify-changed-files`](https://github.com/tj-actions/verify-changed-files) workflow returns the list of files changed within a workflow execution. This could potentially allow filenames that contain special characters such as `;` which can be used by an attacker to take over the [GitHub Runner](https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners) if the output value is used in a raw fashion (thus being directly replaced before execution) inside a `run` block. By running custom commands, an attacker may be able to steal secrets such as `GITHUB_TOKEN` if triggered on other events than `pull_request`.

This has been patched in versions [17](https://github.com/tj-actions/verify-changed-files/releases/tag/v17) and [17.0.0](https://github.com/tj-actions/verify-changed-files/releases/tag/v17.0.0) by enabling `safe_output` by default and returning filename paths escaping special characters for bash environments.
CWE description: The product constructs all or part of a command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended command when it is sent to a downstream component.
CVSS v3 base score: 8.8
Published: 2023-12-29T17:16Z
Repository: https://github.com/tj-actions/verify-changed-files
Language: Shell
Files changed: entrypoint.sh
Commit date: 2023-12-28 19:31:51 -0700
Commit message: chore: update entrypoint.sh (#357)


Diff: diff --git a/entrypoint.sh b/entrypoint.sh
index 6d15487..f580faa 100755
--- a/entrypoint.sh
+++ b/entrypoint.sh
@@ -37,21 +37,30 @@ else
   UNSTAGED_DELETED_FILES=$(git ls-files --deleted | awk -v d="|" '{s=(NR==1?s:s d)$0}END{print s}')
 fi
 
 CHANGED_FILES=""
 
-# Function to concatenate non-empty strings with a separator
+# Function to concatenate non-empty file names with a separator
 concatenate() {
   local separator=$1
   shift
   local result=""
-  for str in "$@"; do
-    if [[ -n $str ]]; then
+  for filename in "$@"; do
+    if [[ "$INPUT_SAFE_OUTPUT" == "true" ]]; then
+      filename=${filename//$/\\$} # Replace $ with \$
+      filename=${filename//\(/\\\(} # Replace ( with \(
+      filename=${filename//\)/\\\)} # Replace ) with \)
+      filename=${filename//\`/\\\`} # Replace ` with \`
+      filename=${filename//|/\\|} # Replace | with \|
+      filename=${filename//&/\\&} # Replace & with \&
+      filename=${filename//;/\\;} # Replace ; with \;
+    fi
+    if [[ -n $filename ]]; then
       if [[ -n $result ]]; then
-        result+="$separator$str"
+        result+="$separator$filename"
       else
-        result="$str"
+        result="$filename"
       fi
     fi
   done
   echo "$result"
 }
@@ -60,34 +69,23 @@ concatenate() {
 CHANGED_FILES=$(concatenate "|" "$TRACKED_FILES" "$UNTRACKED_OR_IGNORED_FILES" "$UNSTAGED_DELETED_FILES")
 
 CHANGED_FILES=$(echo "$CHANGED_FILES"  | awk '{gsub(/\|/,"\n"); print $0;}' | sort -u | awk -v d="|" '{s=(NR==1?s:s d)$0}END{print s}')
 
 if [[ -n "$CHANGED_FILES" ]]; then
-  echo "Found uncommited changes"
+  echo "Found uncommitted changes"
 
   CHANGED_FILES=$(echo "$CHANGED_FILES" | awk '{gsub(/\|/,"\n"); print $0;}' | awk -v d="$INPUT_SEPARATOR" '{s=(NR==1?s:s d)$0}END{print s}')
 
-  if [[ "$INPUT_SAFE_OUTPUT" == "true" ]]; then
-    CHANGED_FILES=${CHANGED_FILES//$/\\$} # Replace $ with \$
-    CHANGED_FILES=${CHANGED_FILES//\(/\\\(}} # Replace ( with \(
-    CHANGED_FILES=${CHANGED_FILES//\)/\\\)}} # Replace ) with \)
-    CHANGED_FILES=${CHANGED_FILES//\`/\\\`} # Replace ` with \`
-    CHANGED_FILES=${CHANGED_FILES//|/\\|} # Replace | with \|
-    CHANGED_FILES=${CHANGED_FILES//&/\\&} # Replace & with \&
-    CHANGED_FILES=${CHANGED_FILES//;/\\;} # Replace ; with \;
-  fi
-
   echo "files_changed=true" >> "$GITHUB_OUTPUT"
   echo "changed_files=$CHANGED_FILES" >> "$GITHUB_OUTPUT"
 
   if [[ "$INPUT_FAIL_IF_CHANGED" == "true" ]]; then
     if [[ -n "$INPUT_FAIL_MSG" ]]; then
       echo "::error::$INPUT_FAIL_MSG"
     fi
     exit 1
   fi
-
 else
   echo "No changes found."
   echo "files_changed=false" >> "$GITHUB_OUTPUT"
 fi
 

Vulnerable code: # Function to concatenate non-empty strings with a separator
  for str in "$@"; do
    if [[ -n $str ]]; then
        result+="$separator$str"
        result="$str"
  echo "Found uncommited changes"
  if [[ "$INPUT_SAFE_OUTPUT" == "true" ]]; then
    CHANGED_FILES=${CHANGED_FILES//$/\\$} # Replace $ with \$
    CHANGED_FILES=${CHANGED_FILES//\(/\\\(}} # Replace ( with \(
    CHANGED_FILES=${CHANGED_FILES//\)/\\\)}} # Replace ) with \)
    CHANGED_FILES=${CHANGED_FILES//\`/\\\`} # Replace ` with \`
    CHANGED_FILES=${CHANGED_FILES//|/\\|} # Replace | with \|
    CHANGED_FILES=${CHANGED_FILES//&/\\&} # Replace & with \&
    CHANGED_FILES=${CHANGED_FILES//;/\\;} # Replace ; with \;
  fi


Fixed code: # Function to concatenate non-empty file names with a separator
  for filename in "$@"; do
    if [[ "$INPUT_SAFE_OUTPUT" == "true" ]]; then
      filename=${filename//$/\\$} # Replace $ with \$
      filename=${filename//\(/\\\(} # Replace ( with \(
      filename=${filename//\)/\\\)} # Replace ) with \)
      filename=${filename//\`/\\\`} # Replace ` with \`
      filename=${filename//|/\\|} # Replace | with \|
      filename=${filename//&/\\&} # Replace & with \&
      filename=${filename//;/\\;} # Replace ; with \;
    fi
    if [[ -n $filename ]]; then
        result+="$separator$filename"
        result="$filename"
  echo "Found uncommitted changes"