metamask-mobile
metamask-mobile copied to clipboard
[FEAT] Screenshot Warning
Description
Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions, 1. What is the reason for the change?
Improve the security around SRP reveal.
2. What is the improvement/solution?
iOS specific. This PR adds a warning when the user takes a screenshot of their SRP or Private Key.
Changes:
- [ ] ~Introduction of
react-native-detectorv0.2.1 package~ - [x] Refactor of
RevealPrivateCredentialand move styles to a different file - [x] Refactor of
ManualBackupStep1and move styles to a different file
it also refactors the component RevealPrivateCredential and ManualBackupStep1
Screenshots/Recordings
If applicable, add screenshots and/or recordings to visualize the before and after of your change
https://user-images.githubusercontent.com/17601467/178645859-b41bff01-b25b-4fa3-8d74-39a326c4894a.MP4
Note: You can observe a similar behaviour in the onboarding flow.
Issue
Progresses https://github.com/MetaMask/mobile-planning/issues/227
Checklist
- [x] There is a related GitHub issue
- [x] Tests are included if applicable
- [ ] Any added code is fully documented
Is the goal of this change to warn the user before they take a screenshot or simply to warn them whenever they take one? Would it even be possible to prevent a screenshot when the user takes one?
https://user-images.githubusercontent.com/22918444/180300751-2862a9b2-edfb-4bc7-aae0-227208c20eef.MOV
@owencraston in response to https://github.com/MetaMask/metamask-mobile/pull/4670#issuecomment-1191861490
The goal is just to warn the user about their action and state explicitly that we don't recommend it. In iOS you can't prevent the screenshot.
@gantunesr thanks for clarifying. I see how that it's not possible on iOS. I am curious how we went about disabling screenshots on Android? is it something we disable only on one screen or is it app wide? if we are disabling it on one screen then we should try and consolidate the logic since these packages allow us to disable screenshots on android.
@owencraston regarding https://github.com/MetaMask/metamask-mobile/pull/4670#issuecomment-1197365885
I am curious how we went about disabling screenshots on Android?
We created a native module to disable it. You can check it here
Is it something we disable only on one screen or is it app wide?
Only SRP or Private Key related screens
if we are disabling it on one screen then we should try and consolidate the logic since these packages allow us to disable screenshots on android.
I support this decision, but it would be out of scope for this development
if we are disabling it on one screen then we should try and consolidate the logic since these packages allow us to disable screenshots on android.
I support this decision, but it would be out of scope for this development
Looking at the expo package screen capture package, it seems like we could easily leverage this package to do both the screenshot prevention and the screenshot listener.
Also, slightly unrelated but it seems we can prevent screen recording on iOS for certain screens.

@gantunesr I did a quick run through to see how this feature works and I received a [TypeError: undefined is not an object (evaluating '_this.props.passwordSet')] when I attempted to reveal my SRP.
Socket Security Pull Request Report
👍 No dependency changes detected in pull request
Pull request report summary
| Issue | Status |
|---|---|
| Install scripts | ✅ 0 issues |
| Native code | ✅ 0 issues |
| Bin script confusion | ✅ 0 issues |
| Bin script shell injection | ✅ 0 issues |
| Unresolved require | ✅ 0 issues |
| Invalid package.json | ✅ 0 issues |
| HTTP dependency | ✅ 0 issues |
| Git dependency | ✅ 0 issues |
| Potential typo squat | ✅ 0 issues |
| Known Malware | ✅ 0 issues |
| Telemetry | ✅ 0 issues |
| Protestware/Troll package | ✅ 0 issues |
Powered by socket.dev
According to socket.dev, this dependency was flagged as deprecated and has some high severity issues like install scripts. I have to take another look at this development.

QA Build 972 crashes on Android 13, 12 and 11 when I attempt to access SRP with a device that uses PIN authenticaton with NO biometric auth factors present on the device. I also noticed that when I attempt to reveal the private key, I get "Incorrect Password" even though I am using the correct password (not shown in video, but also easy to reproduce)
https://recordit.co/Uh75WoiAem
IOS 15.6.1 (edited to add iOS 16 shows this same error) using a device with biometrics present (but not used by MetaMask) I get this error on QA build 972 when I attempt to reveal SRP:

Copy updated according to https://github.com/MetaMask/mobile-planning/issues/227#issuecomment-1307108805 in commit e951e16
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.