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

Undefined behaviour in loadDefaults in FIRCLSUserDefaults.m #5454

Closed
C0lumbo opened this issue Apr 24, 2020 · 1 comment · Fixed by #5456
Closed

Undefined behaviour in loadDefaults in FIRCLSUserDefaults.m #5454

C0lumbo opened this issue Apr 24, 2020 · 1 comment · Fixed by #5456
Assignees
Milestone

Comments

@C0lumbo
Copy link

C0lumbo commented Apr 24, 2020

[REQUIRED] Step 1: Describe your environment

  • Xcode version: 11.2.1
  • Firebase SDK version: 6.22.0
  • Firebase Component: Crashlytics
  • Component version: 6.22.0
  • Installation method: CocoaPods

[REQUIRED] Step 2: Describe the problem

Running on iOS with UBSan enabled (https://developer.apple.com/documentation/code_diagnostics/undefined_behavior_sanitizer/enabling_the_undefined_behavior_sanitizer?language=objc) I hit a piece of undefined behaviour in Crashlytics which causes UBSan to break into the debugger with an "Invalid Boolean" error.

I believe it's likely to be harmless, but also think it should be fixed to reduce noise, and technically the results of undefined behaviour are somewhat unlimited.

Steps to reproduce:

Enable UBSan (https://developer.apple.com/documentation/code_diagnostics/undefined_behavior_sanitizer/enabling_the_undefined_behavior_sanitizer?language=objc)
Run a project with Crashlytics for the first time.

Observe that UBSan breaks due to an invalid value for isDirectory in loadDefaults. This is because isDirectory is not initialized when it's declared, nor is it initialised during the call to fileExistsAtPath (https://developer.apple.com/documentation/foundation/nsfilemanager/1410277-fileexistsatpath?language=objc) because "If path doesn’t exist, this value is undefined upon return. "

The simplest fix would be to remove isDirectory from the "else if (!fileExists && !isDirectory)" condition changing it to "else if (!fileExists) "

@morganchen12
Copy link
Contributor

Thanks @C0lumbo, we'll try to get this fix into the next release.

@morganchen12 morganchen12 self-assigned this Apr 24, 2020
@paulb777 paulb777 added this to the M70 milestone Apr 24, 2020
@firebase firebase locked and limited conversation to collaborators May 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants