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

Why Was issue #150 merged into the VRT? #256

Closed
bc-dshiv opened this issue Aug 8, 2019 · 16 comments · Fixed by #259
Closed

Why Was issue #150 merged into the VRT? #256

bc-dshiv opened this issue Aug 8, 2019 · 16 comments · Fixed by #259

Comments

@bc-dshiv
Copy link

bc-dshiv commented Aug 8, 2019

Issue
We merged Copy/Paste on Sensitive content. The specific issue that brought this up, Issue #150 ends with a comment about why adding this entry is a bad idea. It also links to discussions for why OWASP also did not add this.

The end result here is that users are encouraged to always some how type out their passwords. Regardless if it's possible to copy something from an App. The actual attack is that a random malicious app is able to read the clip board. We allow pasting to the sensitive content though. Okay that would mean the malicious app already has the sensitive data. For a password manager it would make sense the user would naturally being coping data out of it. When does a user normally copy sensitive data out of an app??? Personally I don't remember a need to copy CC# or SSN from one app to another. For passwordsIf I copy it from an alternative source already. A malicious app would get the same access to the data regardless if I can copy the data out of the random app.

Github Issue Comment from OWASP
Users will copy their password or other information only to find that they cannot paste it. So the recommendation we are having is not helping and only making the user experience worse and you are right saying that users might choose simple passwords because of that. So it actually has a bad side-effect to other security controls (password strength).

Recommendation
We should remove this from the VRT as it does not actually help protect mobile apps specifically.

@arcwhite
Copy link
Member

arcwhite commented Aug 9, 2019

Strongly agree.

@dapperrobotbear
Copy link

From a UX perspective I strongly agree. Disabling paste functionality for secure entry fields does not increase security in a form, as the act of copying has already occurred when the user discovers that pasting is not possible. This increases friction and we should not be encouraging developers/organizations to "fix" this issue under the guise that it makes them more secure.

If there is in fact a vulnerability in the paste board allowing actors to access it outside of the app context when a user access it, then the report should be on that component instead.

@Dshiv
Copy link

Dshiv commented Aug 9, 2019 via email

@plr0man
Copy link
Contributor

plr0man commented Sep 3, 2019

This issue was discussed internally and the team decided that P5 is in fact a better fit

@0xdevalias
Copy link

Personal /2c, I think P5 is even too much for this. It implies this functionality is bad, but not bad enough to be of higher risk. Whereas I would argue this functionality in most situations is actually good, and promotes use of better practices (eg. password managers/strong passwords)

@dapperrobotbear
Copy link

Agreed.

@bc-dshiv
Copy link
Author

@0xdevalias @dapperrobotbear Then would your recommendation be to remove it from the VRT entirely? I like the idea you gave. P5 still having value fells 100% correct. Even P5 should actually be considered a good change over all.

@plr0man I believe this should be considered further.

@codingo
Copy link
Member

codingo commented Sep 10, 2019

@bc-dshiv, I strongly agree with @0xdevalias and @dapperrobotbear on this one.

I treat secret questions and answers like passwords. I manage them in a key vault, and I never put real data into them. This feature, if applied, would likely be used to restrict this, creating more of a barrier for users who do the same (a high number of practitioners in the field).

Conversely, an attacker who has access to the clipboard, and manages to highjack the secret question/answer can already do so a variety of other ways through the DOM. There is no tangible risk here that I can see that isn't already countered by something else that the VRT addresses and I feel to justify this as a P5 you need to be able to put some value to that statement.

With that in mind, what's an attack vector here that's not already satisfied by another item that justifies its presence?

@bc-dshiv
Copy link
Author

I also use a vault of secret questions and answers. :D Makes talking with support over an account really fun when I need to tell them an answer.

@bc-dshiv
Copy link
Author

bc-dshiv commented Sep 10, 2019

Mostly this category was in regards to Phone Apps. All Apps by default have access to the clipboard, I believe. I do know that the risk the Team was considering is that a Victim copies data out of an APP. So the only attack scenarios, I see as valid, is when an App is showing sensitive data that the user did not input themselves.

So maybe a health organization has an app they use where patient PII can be copied. Even with that I would much rather the Doctor, nurse, or whomever be 100% able to copy my information accurately.

Over all I 100% believe the risks of forcing users to manually copy items increases the possibility of something going wrong or being bad.

My vote is that it be removed completely.

@0xdevalias
Copy link

@bc-dshiv My personal feels are strongly for removing it entirely. It feels misplaced. Like, yes there is some risk from a malicious app that can access my clipboard doing so, but is the solution to never copy anything? I like to look at security recommendations from the practicality aspects + risk/reward. The likelihood of a rogue app stealing clipboard data is fairly minimal on most modern mobile platforms IMO. Whereas the risk of essentially negatively impacting the UX and thus promoting weaker password/etc use is very real, and has impact beyond the local device.

In writing this, I could see a potential case for a P5 or similarly low rated ‘app does not implement native platforms password manager features’. iOS these days allows direct integration with password managers these days, so you can fill app login fields without copy/paste. I assume android has similar. If that is available in an app, it has the side effect that we wouldn’t have to copy secrets, and increases the UX + security likelihood overall (promotes stronger passwords, etc), with no downside. Note: even if something like that was added, I would still be strongly against this ‘disable copy’ entry.

@0xdevalias
Copy link

0xdevalias commented Sep 10, 2019

For completeness, here is @devinlundberg 's previous objection/comments (on the PR itself):

This should be at most a P5 for sensitive content, but I believe it should be removed from the paid program altogether. This control is a reasonable defense in depth for some very sensitive use cases that do not care about usability. It does not represent an actual vulnerability unlike many of the other P4s. It can only be taken advantage of with user interaction in conjunction with malware being present.

Malware is more prevalent on PCs and macs than it is on mobile devices and disabling copying/pasting is not a common recommendation for web sites. Web security is far more established than mobile security so we should take some hints from the community there.

Users use the clipboard for legitimate purposes and expect it to work. Adding these restrictions can make apps less usable for real people. There are situations where people need to copy personal/sensitive data between apps or within different views of the same app. There are even use cases like password managers where disabling clipboard usage on sensitive inputs will harm security overall because users will pick weaker passwords when they realize they can't user their copied password.

This was rejected from OWASP's mobile recommendations for these reasons: OWASP/owasp-masvs#117

@plr0man
Copy link
Contributor

plr0man commented Sep 13, 2019

Hi Everyone,

P5 VRT entries are there for a few important reasons, mostly disincentivizing reports of corresponding types of issues as well as providing the ASE's with a guideline. If you look at current P5 entries you will see a broad spectrum of issues ranging in severity from what could be considered N/A to not enough to be a P4 in most situations. With how the bounties are currently run having those is essential so we can reduce noise on our side and help the researchers better spend their time. That being said whether we agree if or when this issue could be informational or if it is a firm N/A, we need to document our decision in the VRT.

@bc-dshiv
Copy link
Author

Certainly I do understand the reasoning. It is very valid. Here are some of my thoughts.

To Be Clear
Please view these as my personal opinion.

  1. Adding entries that are P5 to reduce noise make a huge amount of sense for Bugcrowd themselves. The VRT, while managed by Bugcrowd, is supposed to be a standard. The Readme also states, To arrive at this baseline technical severity rating for a given vulnerability.... The purpose is to describe vulnerabilities and not to give Bugcrowd employees easy methods to close submissions. In our discussions here there are good reasons why removing the clipboard has damaging effects. My opinion here is that these effects are enough to remove the entry. I can also personally verify that this entry and maybe only give me one valid issue and the rest being spam.

  2. I do not believe Bugcrowd should be using VRT entries as their main method for reducing noice. There are, I believe, 119 P5 issues. At this point there are more P5 issues than valid rewardable issues. I feel that the VRT is diverging away from describing valid vulnerabilities and describing a set of things Bugcrowd wants to reduce spam for. I very much like the idea of reducing the spam amount we get however the VRT itself is not the place for that. For the VRT to be accepted more across more platforms, it is my opinion it should remain as vendor agnostic as possible. Bugcrowd UI can easily, and I mean very very easily, maintain a list of more entry types that are considered P5 and show those to researcher's during submission creation. Granted if Bugcrowd does not actually care about other companies or platforms using the VRT then all I have said here is a mute point.

@plr0man
Copy link
Contributor

plr0man commented Sep 20, 2019

To add more context, Bugcrowd disincentivizes P5's unless a customer wishes to opt into receiving all or some P5's. These would typically be companies with a very high attention to security, having the capacity to thoroughly assess the security impact. We are not in a position to make the decision about the eligibility of these issues for those customers. In case of the clipboard issue, they may decide that there should not be a copy and/or paste functionality in some particular use case, or that the accessibility service autofill on Android > 5.0 or autofill framework on Android > 8.0 or other password manager supported ways to fill passwords are sufficient to let the users keep their passwords out of their clipboards.
We found that this is the best approach to P5's in general, but of course another organization could find that there's a custom way that works better for them. Anyone is free to start their own fork of the VRT. We on our end are responsible for providing a guideline and documenting our learnings, and not putting the results of this discussion in the VRT is almost as good as never having this discussion at all.
I will verify with the VRT council members if we should circle back to this issue here or if we're good to merge.

@0xdevalias
Copy link

0xdevalias commented Sep 21, 2019

personal /2c: Strongly disagree with this action. Poor choice, ignoring the wisdom of the security community at large (eg. see OWASP's decision that this isn't an issue) for at best arbitrary personal reasons. But, your project and your decisions to make, so ultimately not something we can change I guess.

constructive edit: if the desired effect is to OOS things, maybe just add a new category that isn't 'P5' that is just OOS/NA and collect those things there. Keeps the effect of documenting them/ability to automagically OOS them for bounties, but doesn't imply that they are a security issue when they aren't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

7 participants