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.

Sign up for GitHub

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

Jump to bottom

Fix MitM vulnerability #2132

Merged
merged 2 commits into from
Aug 25, 2021
Merged

Fix MitM vulnerability #2132

merged 2 commits into from
Aug 25, 2021

Conversation

andreidubov
Copy link
Contributor

@andreidubov andreidubov commented Aug 23, 2021 •
edited
Loading

Compare path with the target directory instead of “.”

@andreidubov andreidubov self-assigned this Aug 23, 2021
@andreidubov andreidubov merged commit a9d5e76 into master Aug 25, 2021
@andreidubov andreidubov deleted the v-anddub/fix-msrc-vulnerability branch August 25, 2021 07:04
@Minishlink
Copy link

@andreidubov Looks like it broke bundle signature verification. Rollbacking to v7.0.1 works.

When logging path of copied files, I noticed strange file paths, as if they are nested inside each other :

Copy node_modules_reactnavigation_stack_src_views_assets_backicon.png to /data/user/0/XXX/files/CodePush/1e91b0d65703ea5aaebc183504b71e88eda7cdb715bb07519eb1d3d314e039e5/data/user/0/XXXX/files/CodePush/unzipped/CodePush/drawable-xxxhdpi/node_modules_reactnavigation_stack_src_views_assets_backicon.png

Copy index.android.bundle to /data/user/0/XXX/files/CodePush/1e91b0d65703ea5aaebc183504b71e88eda7cdb715bb07519eb1d3d314e039e5/data/user/0/XXX/files/CodePush/unzipped/CodePush/index.android.bundle

Copy .codepushrelease to /data/user/0/XXX/files/CodePush/1e91b0d65703ea5aaebc183504b71e88eda7cdb715bb07519eb1d3d314e039e5/data/user/0/XXX/files/CodePush/unzipped/CodePush/.codepushrelease

So I wonder if it is what you wanted, if so getSignatureFilePath needs to be tweaked because it looks like it only returns /data/user/0/XXX/files/CodePush/.codepushrelease .

@immutable-pro
Copy link

@andreidubov Looks like it broke bundle signature verification. Rollbacking to v7.0.1 works.

When logging path of copied files, I noticed strange file paths, as if they are nested inside each other :

Copy node_modules_reactnavigation_stack_src_views_assets_backicon.png to /data/user/0/XXX/files/CodePush/1e91b0d65703ea5aaebc183504b71e88eda7cdb715bb07519eb1d3d314e039e5/data/user/0/XXXX/files/CodePush/unzipped/CodePush/drawable-xxxhdpi/node_modules_reactnavigation_stack_src_views_assets_backicon.png

Copy index.android.bundle to /data/user/0/XXX/files/CodePush/1e91b0d65703ea5aaebc183504b71e88eda7cdb715bb07519eb1d3d314e039e5/data/user/0/XXX/files/CodePush/unzipped/CodePush/index.android.bundle

Copy .codepushrelease to /data/user/0/XXX/files/CodePush/1e91b0d65703ea5aaebc183504b71e88eda7cdb715bb07519eb1d3d314e039e5/data/user/0/XXX/files/CodePush/unzipped/CodePush/.codepushrelease

So I wonder if it is what you wanted, if so getSignatureFilePath needs to be tweaked because it looks like it only returns /data/user/0/XXX/files/CodePush/.codepushrelease .

Mentioned here: #2141

@ball-hayden
Copy link

ball-hayden commented Oct 4, 2021 •
edited
Loading

Given when it was added, I wonder if this is also responsible for #2138 (and #2136)?

@ball-hayden ball-hayden mentioned this pull request Oct 5, 2021
Merged
sourabhdebnath pushed a commit to scripbox/react-native-code-push that referenced this pull request May 2, 2022
@andreidubov
Fix MitM vulnerability (microsoft#2132)
2c5460f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexChernyatiev alexChernyatiev alexChernyatiev approved these changes

@n-kurdiukov-akvelon n-kurdiukov-akvelon n-kurdiukov-akvelon approved these changes

@Krasavinigor Krasavinigor Krasavinigor approved these changes

@alexandergoncharov-zz alexandergoncharov-zz alexandergoncharov-zz approved these changes

Assignees

@andreidubov andreidubov

Labels
android
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@andreidubov @Minishlink @immutable-pro @ball-hayden @alexandergoncharov-zz @Krasavinigor @n-kurdiukov-akvelon @alexChernyatiev