search

apptentive / apptentive-ios

Apptentive Legacy SDK for iOS. See https://github.com/apptentive/apptentive-kit-ios for Version 6.
http://www.apptentive.com/
BSD 3-Clause "New" or "Revised" License
146 stars 103 forks source link

ApptentiveLogMonitor calls UIPasteboard in production builds #274

Closed AlexTrott closed 4 years ago

AlexTrott commented 4 years ago

Like many others I have updated to the latest iOS 14 beta, and I noticed that my apps which utilize Apptentive, are all showing the new pasteboard dialog.

I did some digging, and found that ApptentiveLogMonitor looks to be doing weird stuff, which I'd suspect should be behind a debug compiler flag.

If ApptentiveLogMonitor fails to get a session, which it is currently doing on my application, it will attempt to do this weird code, which personally, i'd have behind a debug compiler flag, as it shouldn't be inside production applications.

https://github.com/apptentive/apptentive-ios/blob/4c003b7c0ebc0a95e731ca1aed3e739b56fced46/Apptentive/Apptentive/Misc/ApptentiveLogMonitor.m#L72-L83

For reference I am currently using version 5.2.7 of the SDK.

Can this code be removed completely? or can we at the very least throw it behind a compiler flag?

CaseyApptentive commented 4 years ago

Hi @AlexTrott ! Thanks for the report. Let me discuss this with my team. We'll get back to you with next steps as soon as possible.

If this is urgent and impacting a release, could you be sure to email support@apptentive.com and let us know which app you're working on? That way I can keep your Account Manager in the loop to ensure everything is handled.

Thanks again.

CaseyApptentive commented 4 years ago

@AlexTrott apologies -- I misread your message.

We'll keep this in mind as we ensure our SDK is ready for iOS 14 and will keep you posted!

Let me know if there's anything else I can do in the meantime.

AlexTrott commented 4 years ago

Hey @CaseyApptentive, thanks for getting back to me. I appreciate this change looks like it only really has impact for iOS 14 users, but it affects all OS versions. I have concerns that if ApptentiveLogMonitor fails to initialize, a malicious user can now send request to your /debug/verify endpoint, and potentially do damage.

https://github.com/apptentive/apptentive-ios/blob/4c003b7c0ebc0a95e731ca1aed3e739b56fced46/Apptentive/Apptentive/Misc/ApptentiveLogMonitor.m#L172-L192

Right now iOS 13 and 14 are both failing to initialize ApptentiveLogMonitor 100% of the time, I ran a build against a jailbroken device and I found that this chunk of code always return nil, and when debugging it, I noticed that the .cfg file was nowhere to be seen, the file path doesn't exist anywhere:

https://github.com/apptentive/apptentive-ios/blob/1be524208ac5820b101b662a325666fae181536c/Apptentive/Apptentive/Misc/ApptentiveLogMonitorSession.m#L213-L217

Today I plan on investigating if on the latest version of the SDK, fixes the issue with ApptentiveLogMonitor, and hopefully it does, as after all that is the reason I found this issue, but I still believe debug code should not be in release builds of the framework

AlexTrott commented 4 years ago

@CaseyApptentive just created a quick sample project and it still happens on the latest SDK, and apptentive-log-monitor.cfg isn't in the directory the code is looking for

image
scier commented 4 years ago

This is going to create a user trust and PR problem for potentially all apps using Apptentive, even well before the release of iOS 14; I'd request prioritizing a fix for this issue in the short term, rather than considering this a "fix for iOS 14 release" issue.

AlexTrott commented 4 years ago

@scier Completely agree, this is an issue that was unearthed by iOS 14, but affects all versions. Sounds like ApptentiveLogMonitor isn't initializing for you either. 

SteveThePilot commented 4 years ago

I agree - this needs to be looked at and fixed immediately as this code is used by a lot of big apps that will start to lose users from lack of trust as they start using iOS 14 beta and it becomes blatantly clear to them that something wrong is happening.

CaseyApptentive commented 4 years ago

Thanks, everyone. Our engineering team is looking at solutions now.

Stay tuned for a release with a fix. Will update here when ready.

frankus commented 4 years ago

The pasteboard checking is now disabled (in version 5.2.12) except for devices that have a particular configuration profile installed.