search

newrelic / nr1-github

NR1 Github allows you to create more context to your entities by having access to the GitHub repository, contributors and README.
https://github.com/newrelic/nr1-github/discussions
Apache License 2.0
19 stars 27 forks source link

Github token security issue #71

Closed aswanson-nr closed 3 years ago

aswanson-nr commented 3 years ago

We had a report of a vulnerability in this nerdpack that we'll want to resolve. please refer to the internal JIRA issue for specifics

Acceptance Criteria

moonlight-komorebi commented 3 years ago

i dropped this question in slack but ill also drop it here:

hmmmmm. this is interesting. for the ticket i worked on, one of the reproduction steps was to adjust the url via manual request rather than interaction through the UI. is it possible to invalidate the PAT if the github url is changed outside of the nerdlet? this seems only possible within the app, since we dont have other domain code.

douglasday commented 3 years ago

@nr-kkenney , the reproduction step about adjusting the URL via a manual request was just to bypass any client-side UI checks on the protocol. In this issue, the attacker is still updating the github url in the nerdlet; just using a proxy tool like Burpsuite to bypass any client-side checks.

rudouglas commented 3 years ago

72 should fix this issue

rudouglas commented 3 years ago

This has now been pushed to production :)

rudouglas commented 3 years ago

I've reopened this as i'm meeting with Emily tomorrow to get clarity on the scenario so we are 100% clear on how to resolve this

rudouglas commented 3 years ago

Met with Emily to clarify the scenario which I was then able to reproduce:

  1. User A logs in, navigates to any app -> GitHub Repo (in sidebar)
  2. User B logs in (Incognito Window), navigates to same app -> GitHub Repo
  3. User A enters a PAT and saves it
  4. User B without refreshing the page, enters a custom GHE URL and sets it (which sets globally for the NerdPack)
    • The PAT input shows empty for them and they leave it empty
  5. User A simply refreshes their app
    • Their PAT gets sent to the URL that User B set

77 Should fix this, on initial setup it won't allow you to set an GHEnterprise URL until you have set a PAT. User B is forced to set a PAT which overwrites User A's PAT, so when User A refreshes, it's User B's PAT that gets sent to the custom URL