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

Add entry for - Mobile Security Misconfiguration > Copy/Paste Sensitive Data to Global Clipboard #150

Closed
jquinard opened this issue Apr 16, 2018 · 11 comments
Milestone

Comments

@jquinard
Copy link

We currently don't have an entry for this vuln type. The idea is that if a mobile app allows copy/paste functionality on sensitive inputs (such as CC/SSN) a user could use the copy functionality which would leave the data exposed in the clipboard. Any rogue app on the victim's device can read the clipboard.

The barrier for exploitation is pretty high but there is a simple remediation for this on a per input basis. Initial general consensus for this seems to be P4.

@plr0man plr0man added this to the v1.5 milestone Apr 17, 2018
@ryancblack
Copy link
Contributor

ryancblack commented Apr 20, 2018

I believe this should be included, especially given the prevalence of malicious applications on Android and the relative direct means to prevent this on a per-field basis.

Example mitigation on exampleSensitiveField with callbacks:

exampleSensitiveField.setCustomSelectionActionModeCallback(new ActionMode.Callback() {

  public boolean onPrepareActionMode(ActionMode mode, Menu menu) {
      return false;
  }

  public void onDestroyActionMode(ActionMode mode) {
     return false; 
  }

  public boolean onCreateActionMode(ActionMode mode, Menu menu) {
      return false;
  }

  public boolean onActionItemClicked(ActionMode mode, MenuItem item) {
      return false;
  }
});

@SwayzeSlacks85
Copy link

We are definitely lacking mobile entries in the VRT and I think this would be a good addition. +1 for a P4.

@trimkadriu
Copy link

While this is a good idea to add as an entry in VRT, I think it would be better to separate this in two. So there might be cases where the input content might be sensitive as mentioned (SSN, CC or similar), and there might be cases with non-sensitive content (everyday non-relevant information).
So I would suggest to add it something like below:
P4 -> Mobile Security Misconfiguration -> Enabled Global Clipboard -> Sensitive information
P5 -> Mobile Security Misconfiguration -> Enabled Global Clipboard -> Non-sensitive information
Any thoughts?

@jquinard
Copy link
Author

Wasn't the concern about having a P5 variant that clients may consider fixing them? I don't think non-sensitive copy functionality is even worth Informational.

@trimkadriu
Copy link

If there are non-sensitive information then I do not see any impact or any attack scenario there, so why pushing the client to fix such cases. I think this is what we were following until now in VRT with other entries as well.

@plr0man
Copy link
Contributor

plr0man commented Apr 27, 2018

I tweaked the names a little bit @trimkadriu but it's the same idea. P5 if not an issue and P4 if the content is sensitive.
P4 -> Mobile Security Misconfiguration -> Clipboard Enabled -> On Sensitive Content
P5 -> Mobile Security Misconfiguration -> Clipboard Enabled -> On Non-sensitive Content

@truemongo
Copy link

truemongo commented Apr 27, 2018

I have the same confusion as @trimkadriu, if its admittedly not a security issue (in the case of non-sensitive content), why put it in the VRT at all?
Edit: now I see it was also @trimkadriu that suggested the P5 for non-sensitive information so I'm further confused.

@plr0man
Copy link
Contributor

plr0man commented Apr 27, 2018

There's multiple benefits of doing so e.g.:

  • Setting clear expectations: the non-sensitive variants are explicit and further save time on all sides
  • Triage assistance: ASE's can rely on updating the assigned VRT entry if non-sensitive content is submitted as sensitive or without choosing the variant. Again a simple concept that saves quite a lot of time

We've been using this binary variant concept across the VRT with good results. P5 does not automatically mean Informational, in most cases it is a won't fix. In fact there are multiple N/A's that are classified as P5 for the same reasons as above.

@chrashley-
Copy link

Having it in there explicitly helps ASEs for when a researcher will invariably submit a non-sensitive form, when we have something to push back with that is concrete it helps reduce friction.

plr0man added a commit that referenced this issue May 25, 2018
adamrdavid pushed a commit that referenced this issue May 25, 2018
* Add clipboard enabled on sensitive info

* Fix test issues in #155

* Fix markdown for 'clipboard_enabled'

* Tweak remediation advice for #150
@devinlundberg
Copy link

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

@codingo
Copy link
Member

codingo commented Sep 10, 2019

@devinlundberg I thought I'd make a comment to tag you and point out that a lot of discussion is happening in #256 this past week regarding this. You may want to chime in.

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

No branches or pull requests

9 participants