Fix shell code injection and merging

[blochberger]

The previous code used double quotes to surround paths, which still allows environment variables and shell code to be evaluated by the shell. Hence, we use single quotes now, to avoid this problem.

PoC exploit:

#!/bin/sh -eux

POC=$(mktemp -d)
mkdir -p "$POC"
cd "$POC"
git init
git config difftool.Word.cmd '/path/to/WordGit/diff.js "$LOCAL" "$REMOTE"'

# Test case #1
touch '`touch foo`.docx'
git add ./*.docx
test ! -e foo # Will fail if file 'foo' exists (sanity check)
git difftool -t Word --cached
test ! -e foo # Will fail if file 'foo' exists. Oops.
git reset --hard

# Test case #2
touch  "'"'`touch bar`.docx'"'"
git add ./*.docx*
test ! -e bar # Will fail if file 'bar' exists (sanity check)
ls
git difftool -t Word --cached
test ! -e bar # Will fail if file 'bar' exists. Oops.
git reset --hard

# Cleanup
#rm -rf "$POC"

You need to change the path to WordGit. Then you can run it and test the exit code. If the exit code is 1, the exploit worked. If the exit code is 0 the exploit is fixed.

[Gaelan]

Good catch! Honestly, I'm just happy to see that someone is using this thing.

I think your fix would still allow injection if the filename contained ', though, so we probably need to escape argv somehow.

[blochberger]

You are correct. I amended my change, so that single quotes in file names are now escaped as well. See also updated PoC.

[blochberger]

I also added a fix for issue #1.