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

Remote Code Execution? #216

Closed
dcRUSTy opened this issue Aug 5, 2020 · 18 comments
Closed

Remote Code Execution? #216

dcRUSTy opened this issue Aug 5, 2020 · 18 comments
Assignees

Comments

@dcRUSTy
Copy link
Collaborator

dcRUSTy commented Aug 5, 2020

Describe the bug
Scan a repo with a malicious git.exe in its root. Talisman executes that malicious git.exe binary while scanning on Windows.

To Reproduce
Steps to reproduce the behavior:

  1. Create a malicious program and rename it to git.exe in the root of repo
  2. Now run talisman with --scan or --githook pre-commit
  3. Profit

or

  1. Create a malicious program and rename it to git.exe push commit to a repo.
  2. Now run talisman with --scan
  3. Profit

or

  1. Create a malicious program and rename it to git.exe stage it to a repo.
  2. Now run talisman with --githook pre-commit
  3. Profit

Expected behavior
It should not execute malicious git.exe from repo which it is scanning

Screenshots
talisman_PoC

Desktop (please complete the following information):

  • OS: Windows 10
@harinee
Copy link
Collaborator

harinee commented Aug 6, 2020

@dcRUSTy very interesting. Thank you for raising this.
Just to elaborate on the threats, to understand it better:

1. Create a malicious program and rename it to git.exe push commit to a repo.
2. Now run talisman with --scan
3. Profit

The above threat could be true if an attacker pushed a malicious exe to a repo which you would scan on your pipeline/machine. The malicious exe could therefore get access to perform actions on the CI server/dev machine.

1. Create a malicious program and rename it to git.exe stage it to a repo.
2. Now run talisman with --githook pre-commit
3. Profit

The above threat could take effect if you tried pushing/committing an exe, and it ends up executing on your own machine. Which can be an unintentional attack though. It may not be malicious, but it could still cause a problem if it ends up getting executing.

Sounds right?

@harinee harinee added the bug label Aug 6, 2020
@dcRUSTy
Copy link
Collaborator Author

dcRUSTy commented Aug 6, 2020

Yes...
Regarding second explanation.... another case... the user need not push the exe(assuming malicious EXE is from previous commit) even if a user tries to commit a benign text file, the talisman will kick in on precommit hook which will lead it to starting malicious EXE because for everything talisman calls GIT(.exe)

@svishwanath-tw
Copy link
Collaborator

So the attack vector in case 2 is someone committing a malicious code in order to root/exploit/render-unusable the machine on which development for a repo is taking place ?

@dcRUSTy
Copy link
Collaborator Author

dcRUSTy commented Aug 6, 2020

Yes.
These are all worst case scenarios. The core issue is unintentional execution of code(benign or malicious)

@svishwanath-tw
Copy link
Collaborator

I wonder if all hooking tools suffer from this problem ? Can this be considered acceptable risk for any project that employs git-hooks in Windows development environments ?

@dcRUSTy
Copy link
Collaborator Author

dcRUSTy commented Aug 6, 2020

but what about talisman --scan that functionality is without git-hooks?

@svishwanath-tw
Copy link
Collaborator

That we will definitely fix. CI will be rendered broken because of it.

@dcRUSTy
Copy link
Collaborator Author

dcRUSTy commented Aug 6, 2020

Can this be considered acceptable risk for any project that employs git-hooks in Windows development environments ?

Don't know.... but doesn't sounds good

@svishwanath-tw
Copy link
Collaborator

Can this be considered acceptable risk for any project that employs git-hooks in Windows development environments ?

Don't know.... but doesn't sounds good

On a Windows developer machine where the current working directory is also part of the lookup paths used by the default shell which in this case happens to be cmd.exe.

If a developer tried to commit something in said repo (that has the malicious git.exe) invoking git would enable/trigger/fire the exploit, even before talisman started.

This is why I don't see how this problem can be solved reasonably for the end developer use case.

@dcRUSTy
Copy link
Collaborator Author

dcRUSTy commented Aug 6, 2020

Not the case when dev uses IDE (tried with VS Code/IntelliJ) these IDE's don't use git binary from repo

@svishwanath-tw
Copy link
Collaborator

One of the ways to fix this issue is to use a fixed location to load the git binary.
This will create a problem where GitBash/git is not installed in a standard location on the developers machine. (Maybe they are using scoop, maybe they've installed on D:, F: ,Z: etc)

@dineshba suggested an interesting alternative, ie. use a native go-git library instead of invoking the OS binary.

We will address this issue ASAP.

Thanks for using talisman and reporting this problem @dcRUSTy !

@dcRUSTy
Copy link
Collaborator Author

dcRUSTy commented Aug 6, 2020

How about a check in precommit hook script to ignore git (and curl binary) in the repo.. Not sure but update functionality looks another candidate for this vulnerability as well

@dcRUSTy
Copy link
Collaborator Author

dcRUSTy commented Aug 6, 2020

+1 for git library but is a major rework ... was about to point the same in the next bug report where git.exe output was not formatted properly and leads to command manipulation

@svishwanath-tw
Copy link
Collaborator

svishwanath-tw commented Aug 6, 2020 via email

tinamthomas added a commit to tinamthomas/talisman that referenced this issue Aug 7, 2020
@dcRUSTy
Copy link
Collaborator Author

dcRUSTy commented Aug 7, 2020

PATHEXT

Minor Addition. Not only git.exe. git.com git.bat git.cmd.... etc etc are also possible candidates based on value of PATHEXT variable on Windows.

@prabhu43
Copy link
Contributor

prabhu43 commented Aug 7, 2020

Thanks for the input @dcRUSTy . We will take care of all windows executables

prabhu43 added a commit to prabhu43/talisman that referenced this issue Aug 7, 2020
prabhu43 added a commit to prabhu43/talisman that referenced this issue Aug 7, 2020
prabhu43 added a commit to prabhu43/talisman that referenced this issue Aug 7, 2020
@svishwanath-tw
Copy link
Collaborator

@dcRUSTy We tested and released a fix for checked in git.exe on windows. Please update to the latest release and let us know if it works for you.

@dcRUSTy
Copy link
Collaborator Author

dcRUSTy commented Aug 9, 2020

@svishwanath-tw Looks good to me.. Thanks team 👍

@dcRUSTy dcRUSTy closed this as completed Aug 9, 2020
harinee pushed a commit to harinee/talisman that referenced this issue Aug 9, 2020
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

6 participants