-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
Build React Native Shared Objects with Stack Smash Protection #38018
Conversation
Base commit: 8ddb334 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sadly this change can't be applied as it is. The -fstack-protector
flag is provided the by the Android NDK and we can't just re-apply it. We should fix how we invoke the NDK instead and how we bundle libraries so this is just a workaround fix.
@cortinico I see. Do you suggest that we add this as a cppFlag in build.gradle under |
Nope we need to understand why the NDK is not setting this flag. We don't need to specify it manually |
@cortinico I am not sure if NDK toolchain would automatically set the stack-protector flag by default because adding that flag might cause a size increase and might impact perf negatively . I thought developers are supposed to mention these flags as required. Is there any documentation around this which i can read up on? |
It does. The list of flags that the NDK sets automatically for a release build of 0.72 are:
More on this here also: android/ndk#1693 So |
we are blocked in a huge public program because of the missing stack protection of reactive native. Any forecast by when a version will be available fully supporting this security requirment? |
We'll get back to this after the NDK 25 bump: #37974 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closing as for #36870 (comment)
Summary:
Some of the .so files in React Native does not seem to be stack protected. We figure that out by checking for "__stack_chk_fail" string inside the binary files.
This was probably happening because we were not using the "-fstack-protector" flag.
The fix I have imagined involves adding a "-fstack-protector-all" flag in the compile options in the CMake files.
Changelog:
[ANDROID][ADDED] - Added
-fstack-protector-all
flag in CMakeLists.txt forReactAndroid
andReactCommon
.Test Plan:
One of the ways to test this would be to generate the rn-tester app from this branch and following these steps:
#36870 (comment)