search

coinbase / coinbase-ios-sdk

Integrate bitcoin into your iOS application with Coinbase
https://www.coinbase.com/
Apache License 2.0
171 stars 67 forks source link

`accessToken` storage potentially insecure #40

Closed maxvonhippel closed 2 years ago

maxvonhippel commented 6 years ago

The demo app currently stores the accessToken in NSUserDefaults. If the user backs their device up to their laptop or iCloud without turning on encryption, the token can be reverse engineered from the .plist files encoded in that backup. I think this could present a possible attack vector (if the user backs their device up without encryption within 2 hours of app use). I reported this on HackerOne, and was encouraged instead to open an issue here on the repository.

My suggestion is to cut a PR in which a third-party Keychain wrapper such as Lockbox is used to replace the NSUserDefaults writing and reading with Keychain writing and reading for the accessToken. I'm happy to write this myself, but figured I should ask first if:

  1. you agree that this would be a good change to make,
    1. if you are ok with the use of a third party wrapper, and
    2. if so, if you have any opinions about which one to use (I was considering Lockbox but am not enormously opinionated on this topic).
jwitcig commented 2 years ago

Hi @maxvonhippel, this change seems like a great suggestion, and we greatly appreciate the suggestion; however, coinbase-ios-sdk is being deprecated, and so we will be closing this ticket.

Thank you for your contribution.