bugcrowd / vulnerability-rating-taxonomy

Bugcrowd’s baseline priority ratings for common security vulnerabilities
https://bugcrowd.com/vrt
Apache License 2.0
440 stars 83 forks source link

VRT entry for source code disclosure? #126

Closed truemongo closed 6 years ago

truemongo commented 6 years ago

Hello,

I'd like to propose a new entry to the VRT regarding source code disclosure. I'm thinking, for example, of scenarios where /.git folders are left accessible on the server, allowing an attacker to download the entire source code.

These 2 VRT entries are similar/related: P1 > Sensitive Data Exposure > Critically Sensitive Data > Password Disclosure P1 > Sensitive Data Exposure > Critically Sensitive Data > Private API Keys

In many cases, disclosure of the full source code will lead to at least one of the above disclosures, but not necessarily. Even if there are no passwords or private API keys in the source code, the impact is still quite high.

I'd therefore propose something like: P1 > Sensitive Data Exposure > Critically Sensitive Data > Source Code

My only concern is that this wording might lead researchers to think, that companies' public Github repositories, for example, are in scope - not the intention. Open for discussion!

picklepwns commented 6 years ago

I think this would be a fantastic addition as the impact is quite high. I just worry that in the case of code that could be known through other means the impact isn't as high, not sure how that could be added to the wording. For instance if there was a /.git folder for some open source project, with no configuration data or unique code, then that seems less impactful, or a /.git folder for the client side code of a project. Both of those examples would be low on the impact scale.

However if I can dump your unique application code, that is huge. And obviously for real attackers, that is very often one of the short term goals that can give them the head start they need.

Fantastic idea.

jstnkndy commented 6 years ago

I've come across this several times as well. I agree that the severity of the issue is going to depend on the contents of the source code that you are able to access (hardcoded credentials would be a P1); however, I think a baseline would be a good idea because it is clearly an oversight. I'd put a baseline as either a P2 or a P3.

Dshiv commented 6 years ago

I do believe there is good reason to have an addition entry

Reasons In regards to the name I would recommend: P1 > Sensitive Data Exposure > Critically Sensitive Data > Private Source Code. I believe this would make it much easier for researchers to understand that a public program would not qualify.

In regards to the baseline, I'd say it needs to be P1. Many companies that have private Git repositories may also have intellectual property in those repositories. That for a company may have very high impact.

Next Steps Some study of companies that have been impacted by this publicly may help in determining a good place to go with this one.

csimas1 commented 6 years ago

Thanks for bringing this up Mongo.

I'm personally against this entry as a P1. While it is a significant issue worthy of P1, I think it should be in the Varies category mainly because it could be commonly misused for anything that remotely resembles source code disclosure. Also since Bugcrowd ASE's triage P1's with expedience, having this as default P1 could lead to many urgent false positives. I'm open to counter points :)

truemongo commented 6 years ago

Hi @CarlosSimas28, I was personally hesitant to mark it as a "straight" P1, too. However, I think it can be argued that many other entries that are currently marked with a specific priority are also, if you think about it, "Varies". Case in point:

An API key disclosure by itself will also always vary in impact, depending on what service the API key is for, what level of access it has, etc. I would argue that compared to an API key disclosure, a source code disclosure will have more impact to a company. API keys can be rotated, but a loss of intellectual property in the form of source code is not reversible. In any case, I think this situation is common enough that there should be a VRT entry, and if we cannot come to agreement on priority, then having it as "Varies" is better than no entry. :-)

plr0man commented 6 years ago

If we are considering adding a context based entry then we've already considered this idea and this was the consensus:

We've discussed this issue with the team and don't believe the proposed enhancement is a good fit for the VRT at this time. Current "Sensitive Data Exposure" entries seem to provide enough granularity to address various types of data disclosure as a result of source code disclosure. That being said adding a designated "Source Code Disclosure" entry doesn't seem to provide any significant advantage.

But if we figure out an entry that clearly stands for major loss of intellectual property that may or may not include source code per se, I think we could go for P1

truemongo commented 6 years ago

@plr0man, Currently if we find, for example a misconfigured /.git exposing the company's source code, there is no usable VRT entry unless the source code also includes passwords or API keys. In mature companies, the source code itself will usually not have passwords / API keys hardcoded (those will be in configuration files that are not committed to GIT). So, if we find a repository containing thousands of source code files, how do we report it? So again I'd say there needs to be an entry to cover this, but if you'd rather name it "Major Loss of Intellectual Property" then I'd be okay with that too. :)

plr0man commented 6 years ago

VRT entries can be assigned on any level, that being said Sensitive Data Exposure by itself is a context based entry that can be applied to numerous scenarios not addressed by its children. Adding another context based child doesn't seem to provide any significant advantage. I do agree that we should think about a good name/classification for scenarios involving major loss of intellectual property (via source code or not, that's up for discussion).

truemongo commented 6 years ago

I sense there is some resistance to adding new entries to the VRT (not sure why) - the source code disclosure is a case that comes up very often. What is the motivation behind having these two distinct entries then?

Seems they are very similar to each other, and in line with your current position, it seems they should also be removed in favour of the parent category.

Edit: Lets also consider this, which of these scenarios would you find more impactful:

plr0man commented 6 years ago

We could think about merging these ^ entries as they're both credentials, but then again we want to emphasize the "private" part especially in case of API keys (we could tighten the API key definition, but it suffices as-is). Alternatively what if we just had P1 > Sensitive Data Exposure > Critically Sensitive Data instead? That seems to be the best idea, but defining "critical" (or assigning severity) is not something that has proven itself to be understood consistently by the crowd. Limiting misclassification of P1 entries is a major concern.

I sense there is some resistance to adding new entries to the VRT (not sure why) - the source code disclosure is a case that comes up very often.

What is the advantage of adding another context based Sensitive Data Exposure child?

csimas1 commented 6 years ago

@truemongo
I’ve noticed some confusion (myself included) in how the variants are meant to be used. The presence of variants makes people think they must choose one, and if the one they want isn’t present people tend to get confused. However choosing a variant isn’t a requirement. In this case selecting P1 - Sensitive Data Exposure > Critically Sensitive Data would suffice for properly scoring Source Code Disclosure. I believe variants are meant to give people perspective or examples into what can be considered Critically Sensitive Data.

I can understand how from your perspective we may be holding a double standard, Private API Keys = P1 but Source Code Disclosure doesn’t make the list. This may call into question our use of Variants but that’s a whole other discussion and I don’t want to go down that rabbit hole ;-)

Given the severity of Source Code Disclosure, but also the likely hood of a high number of false positives I think this should be given a Varies entry. Yes people could just stop at Sensitive Data Exposure > Critically Sensitive Data for most entries, but many people don’t realize that’s an option.

truemongo commented 6 years ago

@CarlosSimas28 Sorry for the late reply. Ok, I can see your points, my concern about "Varies" entries are that they leave a lot of room for interpretation. I see your concern about false positives, for example in cases where a single source file is leaked, or only client-side source code (which is visible to anyone, anyway). This said, I think you're probably right leaving as "Varies", though also in reply to @plr0man, the advantage of adding another child entry is to add more granularity to statistics (which is useful both to Bugcrowd, the researchers, and the clients) for what is probably a fairly common submission type. Best, mongo

plr0man commented 6 years ago

Edit: Lets also consider this, which of these scenarios would you find more impactful: -I somehow leak Bugcrowd's API key for, say, Intercom, or some other 3rd party service you use; -I somehow leak Bugcrowd's entire source code

I went ahead and discussed this ^ question with our engineers. While the answer will obviously vary depending on the company, particular service API key or the source code disclosure scenario etc, in our case we'd consider Intercom Api key disclosure to have higher overall impact. We would be still comparing P1's though.

the advantage of adding another child entry is to add more granularity to statistics (which is useful both to Bugcrowd, the researchers, and the clients) for what is probably a fairly common submission type.

Agreed, but at the same time there are downsides that outweigh having highly granular metrics or attempting to deliver an all-encompassing taxonomy. The VRT other than being a vulnerability taxonomy lets us communicate to the researchers what types of issues are considered signal as opposed to noise. Adding a new "Varies" entry in this case would likely become a high source of P5s, but would not increase the number of valid reports. To give you an example of how we address this problem please take a look at Server Security Misconfiguration > Directory Listing Enabled. I'm hoping that this will cast some light at the VRT entry vetting process.

On the other hand we all agree that there are cases where source code disclosure is a solid P1, purely as a loss of intellectual property. While it is tempting to have a P1 entry here, there's a particularly high number of variables that would contribute to a great deal of downgrades, which is contrary to how we view the baseline priorities. And so we're back to "Varies".

For those reasons allowing VRT entry assignment at any level, with Sensitive Data Exposure in particular being a vast category has been working out well so far. I am hoping that this is not an obscure feature and would love to hear any general improvement ideas if it is.

Hope this makes sense and if not please feel free to counter:). We really appreciate your feedback @truemongo!

ghost commented 6 years ago

Assuming people really perceive a contradiction in having Password Disclosure and Private API Keys but not something like Private Source Code, maybe an option would be to consolidate all the variants in a single specific entry describing examples of what is considered "Sensitive Data Exposure" : P1 > Sensitive Data Exposure > Critically Sensitive Data > Private Keys, Passwords or Source Code.

This type of entry solves the issue of people thinking that they must choose the last child in a category and also implies that "Source Code" actually refers to sensitive source code (instead of publicly available source code)

plr0man commented 6 years ago

This type of entry solves the issue of people thinking that they must choose the last child in a category

If this is truly being a problem then we should be looking for a general fix, whether it's more education on how the VRT entry assignment works or alternatively adding an Other entry to all VRT parent nodes

ghost commented 6 years ago

@plr0man I didn't mean to say that a new entry should be added just to fix that issue (assuming it is a very common issue). I was suggesting Critically Sensitive Data > Private Keys, Passwords or Source Code to fulfill the requirements described in previous comments. On the other hand, I think the dropdown menu suggesting entries is very clear because it always shows the parent node as an option, although I have certainly noticed that parent nodes are often the last suggestion and that might be the reason people rarely select that type of option.

plr0man commented 6 years ago

On the other hand, I think the dropdown menu suggesting entries is very clear because it always shows the parent node as an option, although I have certainly noticed that parent nodes are often the last suggestion and that might be the reason people rarely select that type of option.

I'm glad to hear that it's not obscure. We do see parent node VRT assignments quite often on our end, so from this perspective it seems to work as intended.

Regarding the suggested Critically Sensitive Data > Private Keys, Passwords or Source Code, I don't believe it solves the problems discussed in previous comments, e.g.:

On the other hand we all agree that there are cases where source code disclosure is a solid P1, purely as a loss of intellectual property. While it is tempting to have a P1 entry here, there's a particularly high number of variables that would contribute to a great deal of downgrades, which is contrary to how we view the baseline priorities. And so we're back to "Varies".

defining "critical" (or assigning severity) is not something that has proven itself to be understood consistently by the crowd. Limiting misclassification of P1 entries is a major concern.

ghost commented 6 years ago

@plr0man If people generally agree that P1 > Sensitive Data Exposure > Critically Sensitive Data > Password Disclosure and P1 > Sensitive Data Exposure > Critically Sensitive Data > Private API Keys should be changed to "Varies", then Critically Sensitive Data > Private Keys, Passwords or Source Code can also be used as a parent category without specific priority. Does that solve all the problems discussed so far?

plr0man commented 6 years ago

If people generally agree that P1 > Sensitive Data Exposure > Critically Sensitive Data > Password Disclosure and P1 > Sensitive Data Exposure > Critically Sensitive Data > Private API Keys should be changed to "Varies"...

I don't believe this is the case here. We certainly do want to keep those as P1 (adding some granularity wouldn't hurt though)

ghost commented 6 years ago

Uhmm, what did you mean when you quoted these paragraphs in your previous comment?

On the other hand we all agree that there are cases where source code disclosure is a solid P1, purely as a loss of intellectual property. While it is tempting to have a P1 entry here, there's a particularly high number of variables that would contribute to a great deal of downgrades, which is contrary to how we view the baseline priorities. And so we're back to "Varies".

defining "critical" (or assigning severity) is not something that has proven itself to be understood consistently by the crowd. Limiting misclassification of P1 entries is a major concern.

I thought you were implying that there are several factors that can easily downgrade that P1 priority and therefore, a "Varies" classification is more appropriate.

plr0man commented 6 years ago

Private API Keys and Password Disclosure being P1s are doing a good job, there might be occasional downgrades e.g. based on key privileges. but nothing that shows potential for making these entries context based ("Varies"). In case of code disclosure on the other hand "Varies" seems to be the best option

ghost commented 6 years ago

Understood, so then what would be a good name for Source code disclosure ("Varies")? I think several options have been discussed but you were also saying that Sensitive Data Exposure has been working well so far.

plr0man commented 6 years ago

Yes Sensitive Data Exposure has been the entry of choice for this type of issues, Sensitive Data Exposure > Critically Sensitive Data would work too. We don't see any significant benefit of adding a designated varies entry at the moment

codingo commented 5 years ago

I'd like to re-raise this discussion based upon 831f56a26ea32636aab6e3fa665c7c62d27799bb150ac9ed083eac8e4be56c3b.

The problem as I see it with the current implementation using a varies tag is there's no P1 tag to force an escalation to triage without manual intervention over BBF, etc'.

For example, let's take the case where a Jenkins instance has been accidentally set to public read. With this kind of an issue the timing is critical, and setting as stated here would leave this in an unclassified state, leaving it at the bottom of the queue for triage. Given triage don't work weekends this could cause some significant delays for these kind of issues raised on a Friday.

I'm inclined to agree with @rsmith31415 that P1 > Sensitive Data Exposure > Critically Sensitive Data > Private Keys, Passwords or Source Code would be the most appropriate home for this, resolving this issue for more seasoned hunters, and also worded in a way to prevent numerous false positives for organisations that have public Github Organisations and the like.

Also tagging @truemongo for fresh feedback in case his stance on this has changed since the original submission.

barnett commented 5 years ago

My thoughts is VRT is an open-source mapping that can be used outside the scope of Bugcrowd and thus when a problem is specific to Bugcrowd’s implementation of VRT, it should be solved elsewhere (not in the core mapping).

We have this specific problem tracked within our platform roadmap and have plans around a Bugcrowd specific implementation ...soon :wink:.