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

injectedJavaScript isn't properly escaped on Android #20365

Closed
LinusU opened this issue Jul 24, 2018 · 8 comments
Closed

injectedJavaScript isn't properly escaped on Android #20365

LinusU opened this issue Jul 24, 2018 · 8 comments
Labels
Component: WebView Related to the WebView component. Platform: Android Android applications. Resolution: Locked This issue was locked by the bot.

Comments

@LinusU
Copy link
Contributor

LinusU commented Jul 24, 2018

Environment

  React Native Environment Info:
    System:
      OS: macOS High Sierra 10.13.5
      CPU: x64 Intel(R) Core(TM) i7-5557U CPU @ 3.10GHz
      Memory: 996.13 MB / 16.00 GB
      Shell: 5.5.1 - /usr/local/bin/zsh
    Binaries:
      Node: 10.5.0 - /usr/local/bin/node
      Yarn: 1.7.0 - /usr/local/bin/yarn
      npm: 6.1.0 - /usr/local/bin/npm
      Watchman: 4.9.0 - /usr/local/bin/watchman
    SDKs:
      iOS SDK:
        Platforms: iOS 11.4, macOS 10.13, tvOS 11.4, watchOS 4.3
      Android SDK:
        Build Tools: 23.0.1, 26.0.3, 27.0.3
        API Levels: 19, 23, 26
    IDEs:
      Xcode: 9.4.1/9F2000 - /usr/bin/xcodebuild
    npmPackages:
      react-native: ^0.56.0 => 0.56.0 
      @fanny-pack/react-native: ^1.0.0-beta.1 => 1.0.0-beta.1 
      @types/react: ^16.4.6 => 16.4.6 
      @types/react-native: ^0.55.26 => 0.55.28 
      react: 16.4.1 => 16.4.1 
    npmGlobalPackages:
      create-react-native-app: 1.0.0
      react-native-cli: 2.0.1

Description

Any JavaScript passed as the injectedJavaScript property on the WebView component on Android isn't properly escaped, and thus can lead to strange SyntaxErrors.

This can be seen here as the raw string is being passed as an javascript:-uri:

public void callInjectedJavaScript() {
if (getSettings().getJavaScriptEnabled() &&
injectedJS != null &&
!TextUtils.isEmpty(injectedJS)) {
loadUrl("javascript:(function() {\n" + injectedJS + ";\n})();");
}
}

Reproducible Demo

https://snack.expo.io/rkSxWhNVm

On iOS, three "12" are correctly showing up. On Android, only the first "12" is printed.

@LinusU
Copy link
Contributor Author

LinusU commented Jul 24, 2018

If anyone needs to work around this in the meantime, you can use this super shitty hack:

npm install --save encode-utf8 base64-js
const encodeUtf8 = require('encode-utf8')
const base64 = require('base64-js')

// ...

/**
 * @param {string} input
 */
function encodeBase64 (input) {
  return base64.fromByteArray(new Uint8Array(encodeUtf8(input)))
}

// ...

const code = `YOUR JAVASCRIPT CODE GOES HERE`
const injectString = `eval(window.atob('${encodeBase64(code)}'))`

<WebView injectedJavaScript={injectString} />

@ghost
Copy link

ghost commented Jul 25, 2018

I agree that this solution is "shitty hack", but if you need to convert to base64
than a better simple way to encode to base64 can be found in react-native-base64 npm package
https://www.npmjs.com/package/react-native-base64

@LinusU
Copy link
Contributor Author

LinusU commented Jul 26, 2018

@eranbaroz I don't think that that is a better solution. That package is only at 0.0.1, and only has 50 downloads per week. It doesn't use anything that's special to the React Native platform either so I don't see why it would be a better approach than the packages I linked.

But the most compelling reason not to use it is that it doesn't work. I actually used that package first, but window.atob failed to decode the result.

@LinusU
Copy link
Contributor Author

LinusU commented Jul 26, 2018

Does anyone know why @react-native-bot marks all my issues as "Old Version", even though I'm running 0.56 which should be the latest? (ping @kelset ☺️)

@kelset
Copy link
Contributor

kelset commented Jul 26, 2018

Hey Linus,
yeah the bot act weirds sometimes. TBH it may be because it parses @types/react-native: ^0.55.26 => 0.55.28 as RN version? 🤔

I'll edit your post and remove the label lets see what happens.

Anyway thanks for reporting this issue, WebView is indeed quite buggy.

@kelset kelset added Component: WebView Related to the WebView component. and removed ⏪Old Version labels Jul 26, 2018
@eranbo
Copy link

eranbo commented Jul 26, 2018

@LinusU in some cases it might be right. But this specific package is only 1 week old or less and published by yours truly (me:)).
Off-course it doesn't use anything special to the RN platform, this is why this package can be used in any other platform/framework that is javascript based and not running in a browser with window.btoa() / window.atob().
The package you linked do require us to link the package to the project and modifies native code, and probably won't work in create-react-native-app application that runs inside expo, only on ejected applications.

@LinusU
Copy link
Contributor Author

LinusU commented Jul 26, 2018

But this specific package is only 1 week old or less and published by yours truly (me:))

Unfortunately, I don't know who you are so I can't attest whether or not the package is stable. That being said, I fail to see how it would be "better" than my approach, please explain ☺️

The package you linked do require us to link the package to the project and modifies native code, and probably won't work in create-react-native-app application that runs inside expo, only on ejected applications.

That's incorrect, the linked package is simply a small, pure javascript dependency. You can see the sources here:

But as I said, your package didn't work, so I would not recommend using that package until at least that is fixed.

edit: regarding the problem with your package; on the top of my head, it seems like it has something to do with Unicode characters. It seems like you are just using charCodeAt whereas I'm first using a proper UTF-8 encoder, and then turning the resulting bytes into base64.

@eranbo
Copy link

eranbo commented Jul 26, 2018

Hi @LinusU

That's incorrect, the linked package is simply a small, pure javascript dependency

This is weird, I guess I got confused with a different thread - you are right, the package does not require to be linked to the project.

it seems like it has something to do with Unicode characters.

Well, this package meant to be simple and mainly used when you need to convert stuff such a simple text characters i.e. Basic authentication header etc.

Unfortunately, I don't know who you are

:) - it's all about the code,
cheers ;)

(Thanks for the feedback - I really appreciate it)

kelset pushed a commit that referenced this issue Aug 13, 2018
Summary:
These changes will fix executing javascript with any special characters, by making use of the `evaluateJavascript` function on Android 4.4+, and by properly escaping the URI on Android <4.4.

Fixes #19611Fixes #20365Fixes #9749Closes #19655Closes #12321

This PR supersedes #19655 by patching the same problem in all the places, and fixing it for Android <4.4 as well.
Pull Request resolved: #20366

Differential Revision: D9242968

Pulled By: hramos

fbshipit-source-id: f2e1abc786ba333dbd8aaa8922e716fd99ec26e0
@facebook facebook locked as resolved and limited conversation to collaborators Aug 9, 2019
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Aug 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Component: WebView Related to the WebView component. Platform: Android Android applications. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants