Issues with documented validation script

[dlorenc]

Hey,

There are a few subtle bugs with the documented verification script: https://docs.codecov.io/docs/about-the-codecov-bash-uploader#validating-the-bash-script

• The VERSION detection should probably just be removed and pinned to master, since the URL is always fixed and users expect it to point to the latest version. Either way, it shouldn't come from the untrusted script, especially not without proper sanitization.
• The VERSION isn't quoted correctly, which means it can lead to other shell issues (but it should just be removed instead of quoted)
• The shasum script (at least on my machine) ignores the specified algorithm during checking. So using three algorithms doesn't really do anything.

I put together a quick example on how the unquoted bash variable can be abused: https://gist.github.com/dlorenc/152ccd0735196d892d21771e4e0f762a

[thomasrockhu]

Hi @dlorenc, thanks for the message here. Regarding VERSION, we cannot yet remove it from the documentation, as the time between committing to the production branch can be minutes before it loads up on our CDN. In that time, the SHAs will not match.

However, would something like this be reasonable for now,

VERSION=$(grep 'VERSION=\".*\"' codecov | cut -d'"' -f2);
for i in 1 256 512
do
 shasum -a $i -c --ignore-missing <(curl -s "https://raw.githubusercontent.com/codecov/codecov-bash/${VERSION}/SHA${i}SUM") ||
 shasum -a $i -c <(curl -s "https://raw.githubusercontent.com/codecov/codecov-bash/${VERSION}/SHA${i}SUM")
done

I also am not sure what you mean by the script ignoring the algorithm. I believe it's specified as shasum -a $i

[zenmonkeykstop]

Another possible workaround would be to pull the latest release version from Github (though you would probably also run into timing problems depending on how releases go):

curl -s https://codecov.io/env > env;    # this also needs checking! but it's not in the SHASUMs right now                  
curl -s https://codecov.io/bash > codecov;                              
VERSION=$(curl --silent "https://api.github.com/repos/codecov/codecov-bash/releases/latest" | grep '"tag_name":' |sed -E 's/.*"([^"]+)".*/\1/')
curl -s https://raw.githubusercontent.com/codecov/codecov-bash/${VERSION}/SHA256SUM > codecov-hashes
shasum -a 256 -c --ignore-missing codecov-hashes                        

[thomasrockhu]

@zenmonkeykstop, I think we will still be running into the timing issue between releases. Regarding the env script, I'm updating the other uploaders to not depend on --ignore-missing as some earlier versions of shasum do not accept it.

[zenmonkeykstop]

In that case, won't it definitely need to be present?

[thomasrockhu]

@zenmonkeykstop, sorry I don't quite understand

[zenmonkeykstop]

The env script is invoked in the same way as codecov, so it would also need to be hashed and verified.

[thomasrockhu]

@zenmonkeykstop oh 100%, but the env script is not always run. In that case, users shouldn't have to pull down another file. Running shasum should verify regardless of whether or not the env script is present (i.e. verify if if it exists, else don't fail).

Right now, running an older version of shasum will error out as --ignore-missing does not exist as an argument. I'm patching the other versions to allow it. Then, I will update and push a new version of the bash script that will add the env SHAs (see this PR)

[PureKrome]

hi @thomasrockhu - could the documentation please be updated to include a full example of how to do this, please? Many of us don't have bash script skills, especially when we come from windows backgrounds. The docs are assuming the reader has some understanding with bash and pipes, etc. For many of us, we just want to grab the script and use it our CI. That, or maybe some more detailed explanations for us non-script people (blush!)

Also, to docs are saying two things:

• Calc the checksum
• verify the checksum

some of us might be mislead and think 'calc the checksum' also includes checking to see if it's ok/valid.

so yeah .. a complete example would be awesome, please 🙏🏻

Note: previously I asked the similar question on SO before I knew about this repo.

[joseph-galindo]

Hey @dlorenc, about this part:

The shasum script (at least on my machine) ignores the specified algorithm during checking. So using three algorithms doesn't really do anything

Assuming you're using shasum 5.84 or newer on MacOS, I also noticed this behavior. The reason is that for checks, the internal implementation of shasum determines the algorithm to use for dynamic "actual" hash generation based on the length of the incoming "trusted"/"expected" hash.

What this means is that if you provide a SHA256 hash for checking, but run shasum -a 512 -c, the request to use the SHA512 algorithm is effectively ignored, and instead a SHA256 hash is generated dynamically and used for the integrity check against your provided "trusted" hash.

The source is here which I think can give more insight: https://perldoc.perl.org/5.18.4/shasum https://perldoc.perl.org/5.18.4/shasum.txt

Mainly

$alg = defined $sum ? $len2alg{length($sum)} : undef;

With that said, I think older versions of shasum do use the algorithm provided by -a directly; the length-based mapping wasn't always implemented. So in general I'd recommend always providing -a to be safe, even though in your case it's likely determining the algorithm based on trusted hash length.

[dlorenc]

Assuming you're using shasum 5.84 or newer on MacOS, I also noticed this behavior. The reason is that for checks, the internal implementation of shasum determines the algorithm to use for dynamic "actual" hash generation based on the length of the incoming "trusted"/"expected" hash.

Thanks! Yep, I was using that version on OSX. Another victim of algorithm agility :)

[PureKrome]

@thomasrockhu Hi Tom - any update on when the docs will get updated, please?

[dinvlad]

Another workaround for this is using

curl -s "https://raw.githubusercontent.com/codecov/codecov-bash/${CODECOV_VERSION}/SHA${i}SUM" | grep codecov

[thomasrockhu]

@PureKrome the docs have been updated, and you can checkout this post for more details.