search

crystal-lang / shards

Dependency manager for the Crystal language
Other
763 stars 100 forks source link

Minor security issue for git dependencies #396

Closed manveru closed 3 years ago

manveru commented 4 years ago

Due to the way git commands are executed (i.e. no Process.run("git", args: [ ... ]) but interpolation), it's possible to execute stuff outside of hooks while parsing git dependencies:

This shard.yml will write ~/.cache/shards/github.com/crystal-lang/crystal-molinillo.git/booya covertly as a simple proof of concept:

name: shards
version: 0.10.0

authors:
  - Julien Portalier <julien@portalier.com>

dependencies:
  molinillo:
    github: crystal-lang/crystal-molinillo
    commit: "e61b200edb735d5676e11cbe56e8353c8b2ced0b$( touch booya )"

targets:
  shards:
    main: src/shards.cr

crystal: 0.34.0

license: Apache-2.0
bcardiff commented 4 years ago

Probably https://github.com/crystal-lang/crystal/pull/9043 will be helpful here.

Or perform some schema validation in the yaml file.

jhass commented 4 years ago

Well, we don't really have a reason to invoke git through a shell, no? let's just pass arguments individually.

waj commented 4 years ago

I guess passing the arguments without shell should work. Note that adding a dependency to a project is not something we could guarantee as secure anyway. You could just execute any command with the postinstall hooks for example.

straight-shoota commented 3 years ago

I think this was fixed by #447 which escapes all CLI arguments with Process.quote.