Prevent code injection in workflow

[mynameistechno]

Courtesy of GitHub Code Scanning.

I haven't tested this solution, mostly just wanted to share this general recommendation as there are a few of these across the organization. We can delete my branch after this is merged/closed.

[github-actions]

Test Results

 2 files  ±0   4 suites  ±0   7m 32s ⏱️ ±0s
15 tests ±0  15 ✅ ±0  0 💤 ±0  0 ❌ ±0 
38 runs  ±0  38 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit a701f38. ± Comparison against base commit 7c9eeae.

[github-actions]

Test jobs for commit a701f38

• Job 80415 on qcs6490
• Job 80416 on qrb2210-rb1

[lool]

@mynameistechno Was there some context from GitHub Code Scanning on why this is problematic? I don't immediately see why the modified approach is safer and I'm curious to read their guidance!

Is there a way for me to receive these notifications about discovered issues directly?

[mwasilew]

Same here. This patch should work when it comes to the purpose of the workflow, however I don't see any difference wrt code injection.

[mynameistechno]

Is there a way for me to receive these notifications about discovered issues directly?

Can you access this? https://github.com/qualcomm-linux/qcom-deb-images/security/code-scanning/2

I was just clicking around the Security tab to better understand it when I came across this reported vulnerability. I need to better understand permissions and alerts around the security features and how to provide access to maintainers.

Going forward, we will be using Semgrep on PRs for this class of scanning anyway. We plan to use GitHub's dependency scanning/dependabot, GitHub's secret scanning, and Semgrep (instead of CodeQL) for code scanning/static analysis.

@njjetha we should submit a PR to this repo that replaces the repolinter workflow with the qcom-preflights-check once we've verified it is working correctly. This is the workflow we plan to ask all repos to run https://github.com/qualcomm/qcom-actions/blob/main/.github/workflows/qcom-preflight-checks.yml . More info at https://github.com/qualcomm/qcom-actions

Was there some context from GitHub Code Scanning on why this is problematic? I don't immediately see why the modified approach is safer and I'm curious to read their guidance!

The solution was suggested by CodeQL. Essentially from what I can gather, when you assign the input to an intermediary variable, it is stored in memory and does not influence the actual script generation: https://securitylab.github.com/resources/github-actions-untrusted-input/

"The expressions inside of ${{ }} are evaluated and substituted with the resulting values before the shell script is run which may make it vulnerable to shell command injection."

[lool]

Is there a way for me to receive these notifications about discovered issues directly?

Can you access this? Code injection

I can! Thanks

Was there some context from GitHub Code Scanning on why this is problematic? I don't immediately see why the modified approach is safer and I'm curious to read their guidance!

The solution was suggested by CodeQL. Essentially from what I can gather, when you assign the input to an intermediary variable, it is stored in memory and does not influence the actual script generation: https://securitylab.github.com/resources/github-actions-untrusted-input/

"The expressions inside of ${{ }} are evaluated and substituted with the resulting values before the shell script is run which may make it vulnerable to shell command injection."

Ok, so indeed, I can see that if one assumes bad input into that input, it could be used to attach the action shell script; I guess we are not worried about untrusted people having the ability to call the workflow with random inputs, but it's a valid thing to defend against, so we can and should be extra careful.

Thanks, interesting situation!

[lool]

FYI this wasn't defined properly; I submitted a fix here:
#83