-
Notifications
You must be signed in to change notification settings - Fork 8k
Description
I reported this issue via the GitHub HackerOne Bug Bounty program #2085800 and was told it was not a security vulnerability and that I should follow up with an issue here. Here is the original report:
Description:
On April 4th, version 2.26.0 of the gh GitHub CLI was released. The release notes (https://github.com/cli/cli/releases/tag/v2.26.0) say that a new field --insecure-storage was added to gh auth login, which should be the only way to opt-in to writing the oauth_token in cleartext to the disk at ~/.config/gh/hosts.yaml.
Due to the logic on this line
Line 231 in fc685f9
| if !secureStorage || setErr != nil { |
Steps To Reproduce:
- confirm using gh past version 2.26.0 (I used 2.32.1)
- Use the Gnome desktop environment
- Open the Passwords and Keys application
- Ensure all keyrings are locked
gh auth loginon commandline- When prompted to unlock the keyring, refuse to unlock
- Follow auth flow to web page and enter device code
- When prompted to unlock the keyring, refuse to unlock
- cat .config/gh/hosts.yml and observe the cleartext oauth_token entry is present
Not all environments have a keyring configured, and those environments will be more vulnerable because they do not have to refuse to unlock. For example, on Replit, my employer, there is an interactive container environment that does not have a Keyring service.
Recommended fix
Import the log package and above line 231 add the following code:
if secureStorage && setErr != nil {
log.Fatalf("gh no longer supports writing secret credentials to a cleartext file by default. To securely store your credentials, unlock or fix your operating system's keyring. If you cannot use the keyring and fully understand the risks, you may pass --insecure-storage to bypass this. error:", setErr)
}
Impact
If the attacker has local access to a computer's file system, they can use the oauth token to read a user's private GitHub repositories and write to all of the repositories they have access to. Additionally, they can do anything the gh cli can do.
Issue
If this is not actually a security vulnerability, then it is a UX issue, because the commandline help says
--insecure-storage Save authentication credentials in plain text instead of credential store
which implies it will not insecurely store credentials without that flag.
The release note also says
We've added a new flag, --insecure-storage, to opt into the old insecure behavior in case you need to debug an issue with credential storage.
which also implies you need to use the new flag to opt into old insecure behavior, but you do not need to use that flag to opt into the behavior.