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

A warning from Apple [resolved, not about React Native] #12778

Closed
XHTeng opened this issue Mar 8, 2017 · 24 comments
Closed

A warning from Apple [resolved, not about React Native] #12778

XHTeng opened this issue Mar 8, 2017 · 24 comments

Comments

@XHTeng
Copy link

@XHTeng XHTeng commented Mar 8, 2017

I received a warning from Apple this morning , how to solve it :

Dear Developer,

Your app, extension, and/or linked framework appears to contain code designed explicitly with the capability to change your app’s behavior or functionality after App Review approval, which is not in compliance with section 3.3.2 of the Apple Developer Program License Agreement and App Store Review Guideline 2.5.2. This code, combined with a remote resource, can facilitate significant changes to your app’s behavior compared to when it was initially reviewed for the App Store. While you may not be using this functionality currently, it has the potential to load private frameworks, private methods, and enable future feature changes.

This includes any code which passes arbitrary parameters to dynamic methods such as dlopen(), dlsym(), respondsToSelector:, performSelector:, method_exchangeImplementations(), and running remote scripts in order to change app behavior or call SPI, based on the contents of the downloaded script. Even if the remote resource is not intentionally malicious, it could easily be hijacked via a Man In The Middle (MiTM) attack, which can pose a serious security vulnerability to users of your app.

Please perform an in-depth review of your app and remove any code, frameworks, or SDKs that fall in line with the functionality described above before submitting the next update for your app for review.

Best regards,

@csbzhixing
Copy link

@csbzhixing csbzhixing commented Mar 8, 2017

Did anyone receive the warning e-mail?

@tdzl2003
Copy link
Contributor

@tdzl2003 tdzl2003 commented Mar 8, 2017

This includes any code which passes arbitrary parameters to dynamic methods such as dlopen(), dlsym(), respondsToSelector:, performSelector:, method_exchangeImplementations(), and running remote scripts

Imported any framework for crash collections?

@XHTeng XHTeng changed the title iOS review warning A warning from Apple Mar 8, 2017
@tdzl2003
Copy link
Contributor

@tdzl2003 tdzl2003 commented Mar 8, 2017

I searched in all code of react native, the only risk which contain "arbitrary parameters to dynamic methods" is:

RCTUtils.m Line 337 & 354

@ide
Copy link
Contributor

@ide ide commented Mar 8, 2017

From the data points I've heard: the developers who received this email were using a patching framework called Rollout.io (discussion here: https://forums.developer.apple.com/thread/73640). So please double-check to see whether your app is using a framework like that. Reading the message carefully, it's not describing React Native nor what React Native does.

Second, make sure that you aren't using custom, third-party React Native modules that call methods like dlopen() and other method calls like the ones listed in the message.

Last, please use the Subscribe button on GitHub if you wish to follow the conversation but don't have something meaningful to contribute. I'd prefer to leave it open but if the discussion isn't productive we'll have to close this issue and perhaps lock it.

Edit: there is also some discussion that the JSPatch library may be triggering this warning. I don't know if that is actually true or not, but please check all of the libraries that you are using in your apps.

@tdzl2003
Copy link
Contributor

@tdzl2003 tdzl2003 commented Mar 8, 2017

I searched in all code of react native, the only risk which may contain "arbitrary parameters to dynamic methods" is:

RCTUtils.m Line 337 & 354

@ide any idea for this? Could we remove or change these code?

@ide
Copy link
Contributor

@ide ide commented Mar 8, 2017

@tdzl2003 Thanks for providing the file and line numbers -- those functions (RCTSwapClassMethods and RCTSwapInstanceMethods) are used only internally within React Native and are not exposed to JavaScript, for example. The way React Native uses them for swizzling is the same as any other "regular" iOS app would. It's very normal, accepted, and not something to be concerned about.

@grabbou
Copy link
Collaborator

@grabbou grabbou commented Mar 8, 2017

3.3.2 of Apple Developer Program License:

An Application may not download or install executable code. Interpreted
code may only be used in an Application if all scripts, code and interpreters are
packaged in the Application and not downloaded. The only exception to the
foregoing is scripts and code downloaded and run by Apple's built-in WebKit
framework or JavascriptCore, provided that such scripts and code do not change
the primary purpose of the Application by providing features or functionality that are
inconsistent with the intended and advertised purpose of the Application as
submitted to the App Store.

explicitly mentions that:

An Application may not download or install executable code

React Native does none of these. And so, using React Native doesn't expose you to the aforementioned issue.

As @ide mentioned, there are reports this message is addressed to those using Rollout.io. My assumption is that it uses aforementioned methods to patch executable code.

With React Native, you can do so called OTA update, but this is updating Javascript, not native code. Whenever native code changes, you have to make a new release. That OTA update of Javascript code is explicitly allowed in the 3.3.2:

The only exception to the foregoing is scripts and code downloaded and run by Apple's built-in WebKit framework or JavascriptCore

I believe everyone should follow up on this issue with Apple and sort out what's exactly causing the warning.

@tdzl2003
Copy link
Contributor

@tdzl2003 tdzl2003 commented Mar 8, 2017

@ide I know they are safe, but maybe apple's automatic check system may still consider a application use both script downloading AND arbitrary parameters to dynamic methods as a risk?

@brunolemos
Copy link
Contributor

@brunolemos brunolemos commented Mar 8, 2017

TL/DR:

🚫 Problem: Apps with the lib Rollout.io, JSPatch and similars are being rejected because they can dynamically change Swift / Objective-C code without passing through Apple Review process.

⚠️ Libraries like Code Push only updates JavaScript code and were not affected, at least not yet.

React Native has nothing to do with this problem, don't worry about it.


rollout-is-aware-screenshot

Apple Developer Program License Agreement

This is the updated terms, from March 01st, 2017. No changes related to javascript code update 👍

apple-store-agreement

@chevins
Copy link

@chevins chevins commented Mar 8, 2017

Good news! My app just passed the review 5 minutes ago. And it contains the 'code-push'. BTW, I don‘t use JSPatch or Bugtags.

@axemclion
Copy link
Contributor

@axemclion axemclion commented Mar 8, 2017

Anyone here using CodePush and had issues with Review ? I am the CodePush PM, so would like to know more if you have run into issues.

@foobra
Copy link

@foobra foobra commented Mar 8, 2017

@axemclion @tdzl2003 CoduRush is also change the App's behavior out of Apple's review.

@axemclion
Copy link
Contributor

@axemclion axemclion commented Mar 8, 2017

@pinguo-gaoshan While CodePush may be misused to change app's behavior, it still cannot load private frameworks or expose private methods other than the ones ReactNative already exposes. Not all the clauses in that rejection apply to CodePush.
Additionally, I would suggest that individual apps should NOT change their behavior using CodePush.

@foobra
Copy link

@foobra foobra commented Mar 8, 2017

@axemclion I agree, but Apple can't distinguish the way developers using CodeRush, so it's bad they all disable these functions, and do not allow new version submit to app store.

@brunolemos
Copy link
Contributor

@brunolemos brunolemos commented Mar 8, 2017

@pinguo-gaoshan it's a possibility that Apple follow this path in the future, but right now apparently code push continues to be accepted.

@axemclion
Copy link
Contributor

@axemclion axemclion commented Mar 8, 2017

@pinguo-gaoshan Note that the rejection explicitly calls out methods that they are concerned about. React Native does not dynamically inject into any of those methods.

@nihgwu
Copy link
Contributor

@nihgwu nihgwu commented Mar 8, 2017

@axemclion I published a brand new app written in React Native with Code Push yesterday without any rejection, and I've made a new version which is still waiting for review, I haven't received any warning email from Apple

@vjeux
Copy link
Contributor

@vjeux vjeux commented Mar 8, 2017

FYI, given the high visibility and uncertainty around this issue, we are actively using the moderation tools in order for it to remain useful. Please try to only reply if you have concrete information around the subject. Thanks!

@huhuanming
Copy link

@huhuanming huhuanming commented Mar 8, 2017

It seems that only Chinese developers received this warning email.

Do other parts of the world have the same email?

@pvenkatakrishnan
Copy link

@pvenkatakrishnan pvenkatakrishnan commented Mar 8, 2017

Ok i noticed on the guidelines, they removed the word javascriptcore alone in 3.3.2
Does it mean dynamically downloading react-native bundles, on the fly, is a no no?
https://developer.apple.com/programs/terms/ios/standard/ios_program_standard_agreement_20140909.pdf

@ide
Copy link
Contributor

@ide ide commented Mar 8, 2017

@huhuanming I believe so, some developers using a library called Rollout received an email. It could be that many Chinese apps use JSPatch, another library used by people who received an email. (Incidentally, both of those libraries were covered by FireEye with regard to security: https://www.fireeye.com/blog/threat-research/2016/04/rollout_or_not_the.html)

@pvenkatakrishnan As @Pikaurd says, that is a very old agreement. The latest one for 2017 (http://adcdownload.apple.com/Documentation/License_Agreements__Apple_Developer_Program/Apple_Developer_Program_License_Agreement_20170227.pdf) has the same language as 2015 and 2016 and mentions JavascriptCore.

@ide
Copy link
Contributor

@ide ide commented Mar 8, 2017

From reading the message from Apple and the data points we've observed, this warning is not about React Native.

On the technical concern of dynamically executing Objective-C described in Apple's email:

Apple's message reads to me that they're concerned about libraries like Rollout and JSPatch, which expose uncontrolled and direct access to native APIs (including private APIs) or enable dynamic loading of native code. Rollout and JSPatch are the only two libraries I've heard to be correlated with the warning.

React Native is different from those libraries because it doesn't expose uncontrolled access to native APIs at runtime. Instead, the developer writes native modules that define some functions the app can call from JavaScript, like setting a timer or playing a sound. This is the same strategy that "hybrid" apps that use a UIWebView/WKWebView have been using for many years. From a technical perspective, React Native is basically a hybrid app except that it calls into more UI APIs.

Technically it is possible for a WebView app or a React Native app also to contain code that exposes uncontrolled access to native APIs. This could happen unintentionally; someone using React Native might also use Rollout. But this isn't something specific to or systemic about React Native nor WebViews anyway

On the strategy of calling native code from JavaScript:

Apple has been fine with WebViews in apps since the beginning of the App Store, including WebViews that make calls to native code, like in the Quip app. WKWebView even has APIs for native and web code to communicate. It's OK for a WebView to call out to your native code that saves data to disk, registers for push notifications, plays a sound, and so on.

React Native is very much like a WebView except it calls out to one more native API (UIKit) for views and animations instead of using HTML.

What neither WebViews nor React Native do is expose the ability to dynamically call any native method at runtime. You write regular Objective-C methods that are statically analyzed by Xcode and Apple and it's no easier to call unauthorized, private APIs.

Data points

The developers who received this email were using Rollout or JSPatch in their apps. People who are using only React Native or libraries/frameworks like CodePush and Expo are not affected and are continuing to have their apps accepted by the App Store review.

Recommendations

If you're writing a React Native or WebView app, be sure not to expose dynamic, uncontrolled access to native APIs and you should be OK. There was one third-party RN module that did this and the maintainers have taken it down. And in accordance with the iOS developer terms, make sure your updates don't change the primary purpose of your app and that the changes are consistent with the intended and advertised purpose of your app.

-- James

@ide ide closed this Mar 8, 2017
@ide ide changed the title A warning from Apple A warning from Apple [resolved, not about React Native] Mar 8, 2017
@facebook facebook locked and limited conversation to collaborators Mar 9, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.