ExodusMovement / lerna-release-action

Selectively release packages from a lerna monorepo
0 stars 0 forks source link

Option injection hardening, src/utils/git.ts #37

Closed mbaraniak-exodus closed 7 months ago

mbaraniak-exodus commented 7 months ago

Description: Several functions defined within src/utils/git.ts are not locking command line options:

export function add(pathSpecs: string[]) {
  spawnSync('git', ['add', ...pathSpecs])
}

export function commit({ message, body, flags }: CommitParams) {
  const args = ['commit', ...flagsAsArguments(flags)]

  if (message) {
    args.push('-m', `"${message}"`)
  }

  if (body) {
    args.push('-m', `"${body}"`)
  }

  spawnSync('git', args)
}

export function resetLastCommit({ flags }: ResetLastCommitParams) {
  spawnSync('git', ['reset', ...flagsAsArguments(flags), 'HEAD~1'])
}

If the attacker ever gains control to invoke the functions listed above with malicious options, it could be possible to jump into interactive shell or perform file inclusion attack.

Steps to reproduce: The easiest way to test git additional command line options will be using your favourite shell:

git commit could be leveraged to jump into interactive shell. Enable interactive editing with -e command line option, and type !/bin/sh

git add  -e
# in the default editor (likely vi or vim) type:
!/bin/sh

git commit additionally could leverage -F option to perform local file include and leak file content in the commit message:

git commit --help

-F <file>, --file=<file>
           Take the commit message from the given file

Some extra information could be found here: https://gtfobins.github.io/gtfobins/git/

Remediation: One potential but not ideal solution would be checking if the first character in each supplied string is not equal to - plus validating if string is [a-zA-Z_\-0-9@/]+

Test note: git commit ``-e will also work, demonstrating that checking only the first character to be not - is not sufficient.

633kh4ck commented 7 months ago

Also, git add --force . could be used to add ignored files (unless the ignored files are enforced by a git hook using git update-index --assume-unchanged <file>), which can lead to the indexing (and potentially commit) of sensitive files.