Validate gradlew scripts

[di72nn]

Also validate gradlew and gradlew.bat scripts.
These are not blobs so it is harder to overlook something, but I think these should be checked too.

[JLLeitschuh]

That doesn't always really work. Some organizations modify their gradlew scripts when updating them in order to, for example, increase the memory the wrapper is given. When doing that, the checksum can't be validated.

https://gradle/gradle:buildSrc/subprojects/versioning/src/main/kotlin/org/gradle/gradlebuild/versioning/WrapperPlugin.kt@298ac44#L34-L42

[di72nn]

That's a fair point, but I still think that there should be an option (or another step in the workflow) to check the scripts for those who don't need to modify them.

[JLLeitschuh]

@eskatos how difficult would it be to also generate SHA checksums for the gradlew and gradle.bat files?
One problem that you'd also run into here is that, unless you had your .gitattributes (for example) file setup correctly, depending upon the OS that created the repository, you may have different line endings (windows vs unix line endings) in the file, thus causing the SHA checksums to be different.

[eskatos]

@JLLeitschuh it shouldn't be difficult. But it'll require work to automate generating the hashes, publishing them where appropriate etc.. You could have a look at how the wrapper jar hashes are handled, it should be similar both in where and how it's done. Then we could update this action to consume them for more validation.

As said above, I wonder what the signal/noise ratio would be given the line endings potential problem and the fact it is intended that people modify it if they need to e.g. change the Gradle client VM options.

[robstoll]

I would vote for an opt-out of the additional check (in the sense of security first).

[vlsi]

As said above, I wonder what the signal/noise ratio would be given the line endings potential problem and the fact it is intended that people modify it if they need to e.g. change the Gradle client VM options.

1. The verifier could normalize line endings (gradlew to LF and gradlew.bat to CRLF) before doing the comparison (it might make sense to normalize to LF always).

2. Frankly speaking, I have not seen a case when users customized gradlew scripts. I assume an opt-out would really help here: the extra skip-gradlew-scripts-validation: true would make it explicit that the files are augmented on purpose.

[tlf30]

I also agree. While some users may modify their scripts, I have not personally seen it before. It would be incredibly nice to have in order to verify the integrity of the scripts. An opt out is a fine solution in my opinion. And I think normalization of line endings would be the way to go as already mentioned.

[magicgoose]

If I were to think like an attacker looking to plant some backdoors or whatever, I may need to choose between:

1. infect wrapper jar:
   a. it is obvious that no one will even try reviewing it by eye, obviously only a movie superhero can do it
   b. already existing tools, like this one, will catch it by a checksum check
2. infect gradlew[.bat] script:
   a. technically it's just a large script, in a somewhat-readable language; given enough time, someone can review it in full with some chance of success, so someone else may also "hope" that someone else had/would have reviewed it
   b. it is likely that no automation will catch a malicious injection in it
   c. people who didn't do research, may assume that their already-set-up automation, like this project, will verify the script too

so my chances of being successful seem to be much higher if I go for the script

[sciencewhiz]

Count me as a +1. I recently saw an attack where someone opened a PR with a malicious gradlew script. It was obvious enough that a reviewer would have noticed, but they exploited another flaw in the workflow that let it run without review. Had the gradlew script been validated, that would have thwarted the attack.

[bigdaz]

@sciencewhiz Thanks for reporting. Can you point to the malicious PR, or share more details about what was attempted? This information would be interesting to the Gradle team, and could help bump the importance of this issue.

Currently there are no checksums generated for wrapper scripts, so it's not so easy to validate the gradlew wrapper scripts.

[sciencewhiz]

The PR has been deleted. The change to the gradle wrapper script was pretty simple. It added a line at the beginning that downloaded a script and executed it, and the script dumped memory looking for secrets.

Like I said originally, a reviewer would have caught it, but there was a separate flaw in the projects workflow that allowed it to execute without review.

[bigdaz]

Thanks @sciencewhiz. This is the first time I've heard of a genuine attack of this sort. Was this for a public, open-source project?

but there was a separate flaw in the projects workflow that allowed it to execute without review.

Fortunately, PRs executed from external forks via the pull_request trigger have restricted permissions and don't have access to GitHub Actions secrets. See here for more details.

If you were using pull_request_target then things get more dangerous, although by default the PR code will not be checked out when using actions/checkout, so you'd have to explicitly checkout the PR branch in order to execute the malicious code.