App Security issues found in Firebase iOS SDK

[Nookaraju]

Step 0: Are you in the right place?

• For issues or feature requests related to the code in this repository file a Github issue.
  • If this is a feature request please use the Feature Request template.
• For general technical questions, post a question on StackOverflow with the firebase tag.
• For general (non-iOS) Firebase discussion, use the firebase-talk google group.
• For backend issues, console issues, and other non-SDK help that does not fall under one of the above categories, reach out to Firebase Support.
• Once you've read this section and determined that your issue is appropriate for this repository, please delete this section.

[REQUIRED] Step 1: Describe your environment

• Xcode version: 11.3
• Firebase SDK version: 6.23.0
• Firebase Component: Anaytics
• Component version: 6.23
• Installation method: `CocoaPods

[REQUIRED] Step 2: Describe the problem

We are using Firebase Analytics in our apps and there are a couple of security vulnerability issues found by our App Security Scanner (Data Theorem).

Steps to reproduce:

1. App Embeds SQL Query with Dynamic Input References: SELECT rowid, FROM %@ WHERE %@ CREATE TABLE IF NOT EXISTS %@ ( SELECT rowid, FROM %@ WHERE %@ = ? INSERT INTO %@ SELECT * FROM %@ UPDATE %@ SET %@ WHERE %@=? About 12 more findings with dynamic queries

2. Hash Generated Using Broken Cryptography API (SHA1) References: -[FIRInstallationsIIDStore sha1WithData:] calls _CC_SHA1()

3. Message Digest Generated Using Broken Cryptography API [$_Unknown_Class (MD5) apm_MD5Data] calls _CC_MD5()

Relevant Code:

// TODO(you): code here to reproduce the problem

[google-oss-bot]

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

[morganchen12]

Action items for the Analytics team:

• Confirm that the SQL query format strings are not populated with user input. (iirc they are not)
• Confirm that SHA1 and MD5 are not used for hashing secrets.

[paulb777]

Actually it's Installations, not Analytics - reassigned.

[maksymmalyhin]

@Nookaraju Thank you for the report.

-[FIRInstallationsIIDStore sha1WithData:] method is used only once per application installation to migrate data from legacy Firebase Instance ID storage. I can confirm that SHA1 hash function was never used for encryption in either the legacy Firebase Instance ID SDK or Firebase Installations SDK. So feel free to ignore this particular warning.

[maksymmalyhin]

apm_MD5Data method defined in Analytics SDK.

@allenktv, @htcgh Could you confirm if it is an actual security issue?

[allenktv]

For Analytics: apm_MD5Data method is used only to calculate fingerprints for uniqueness. User inputs are also sanitized for the SQL database.

[Nookaraju]

@allenktv you are saying issue (App Embeds SQL Query with Dynamic Input References) does not issue?

[maksymmalyhin]

@Nookaraju I'm afraid the SQL issue slipped from our attention. Sorry about that and thank you for bringing this up again. Let us evaluate the potential issue again.

[baolocdo]

@Nookaraju Thanks for the report. Due to the closed source, we can't show you where and how the code sanitize. To clarify for @allenktv 's comment, the %@ is only for hard coded strings (table names, column names) so that we can reuse the code internally. All user inputs are validated, sanitized and bound appropriately for each query. We have been through security audit of the source code before releasing.

[morganchen12]

Closing this based on @baolocdo's comment. Please let us know if you find any more security warnings.