Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lack of integrity verification of downloaded external dependencies #162

Closed
mensfeld opened this issue Apr 1, 2021 · 2 comments
Closed

Comments

@mensfeld
Copy link

mensfeld commented Apr 1, 2021

Hey,

My name is Maciej Mensfeld and I run a research security project called diffend.io.

I've noticed, that this library downloads some external resources and uses them. While it's a totally common pattern, what is lacking here is integrity verification.

You could verify the integrity of the downloaded file before using it by comparing the file hash to a hardcoded, expected file hash.

This is essentially what package managers do to verify the integrity of downloaded packages.

Doing this would prevent attack scenarios in which SwiftLint is manipulated.

Have a great day :)

@ashfurrow
Copy link
Owner

Hi Maciej 👋 You are correct that we download resources from GitHub to execute:

REPO = 'https://github.com/realm/SwiftLint'
VERSION = ENV['SWIFTLINT_VERSION'] || DangerSwiftlint::SWIFTLINT_VERSION
ASSET = 'portable_swiftlint.zip'
URL = "#{REPO}/releases/download/#{VERSION}/#{ASSET}"
DESTINATION = File.expand_path(File.join(File.dirname(__FILE__), 'bin'))
puts "Downloading swiftlint@#{VERSION}"
sh [
"mkdir -p '#{DESTINATION}'",
"curl -s -L #{URL} -o #{ASSET}",
"unzip -q #{ASSET} -d '#{DESTINATION}'",
"rm #{ASSET}"
].join(' && ')

And you are correct that we could hardcode and md5 hash (for example) into our codebase to verify the integrity of SwiftLint. We haven't done so because it hasn't been a priority; we rely on HTTPS to provide reasonable-ish security. For a tool that runs on CI servers, this has been sufficient.

That said, it would not be difficult to add an integrity check (and corresponding unit test). I'm going to leave this issue open in case anyone would like to add this.

NicolasCombe5555 added a commit to NicolasCombe5555/danger-ruby-swiftlint that referenced this issue Aug 30, 2021
NicolasCombe5555 added a commit to NicolasCombe5555/danger-ruby-swiftlint that referenced this issue Aug 30, 2021
NicolasCombe5555 added a commit to NicolasCombe5555/danger-ruby-swiftlint that referenced this issue Aug 31, 2021
NicolasCombe5555 added a commit to NicolasCombe5555/danger-ruby-swiftlint that referenced this issue Aug 31, 2021
NicolasCombe5555 added a commit to NicolasCombe5555/danger-ruby-swiftlint that referenced this issue Sep 2, 2021
ashfurrow added a commit that referenced this issue Sep 7, 2021
Add integrity validation when downloading Swiftlint (#162)
NicolasCombe5555 pushed a commit to NicolasCombe5555/danger-ruby-swiftlint that referenced this issue Sep 7, 2021
ashfurrow added a commit that referenced this issue Sep 7, 2021
Fix "Rake aborted; Command failed with status 127" (#162)
NicolasCombe5555 added a commit to NicolasCombe5555/danger-ruby-swiftlint that referenced this issue Sep 7, 2021
ashfurrow added a commit that referenced this issue Sep 7, 2021
Fix `Command failed with status 127` (#162)
@mensfeld
Copy link
Author

Thank you. Much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants