Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,12 @@ fun applyBinaryDiffPatches(

// Manifest-supplied paths come from the update's JSON, so we treat them as untrusted.
// Resolve them strictly under `base` and reject anything ("../../etc", an absolute path) that would otherwise
// let a manifest entry read or write outside the package/patch folders.
// let a manifest entry read or write outside the package/patch folders. Callers treat the result as a file
// inside `base`, so `base` itself (e.g. from ".") does not pass either.
private fun resolveWithin(base: File, relativePath: String): File {
val baseCanonical = base.canonicalFile
val resolved = File(base, relativePath).canonicalFile
if (resolved != baseCanonical && !resolved.path.startsWith(baseCanonical.path + File.separator)) {
if (!resolved.path.startsWith(baseCanonical.path + File.separator)) {
throw BinaryDiffApplyException(relativePath, "path escapes expected directory")
}
return resolved
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.microsoft.codepush.react.diffpatch

import com.microsoft.codepush.react.CodePushConstants
import org.json.JSONException
import org.json.JSONObject

Expand Down Expand Up @@ -36,15 +37,20 @@ fun parseDiffManifest(json: JSONObject): DiffManifest {
emptyList()
}

val reservedPatchesFolderPrefix = "${CodePushConstants.DIFF_PATCHES_FOLDER_NAME}/"
val patchedFilesJson = json.optJSONObject("patchedFiles")
val patchedFiles = if (patchedFilesJson != null) {
patchedFilesJson.keys().asSequence().associateWith { relativePath ->
val entry = patchedFilesJson.getJSONObject(relativePath)
val patch = entry.getString("patch")
if (!patch.startsWith(reservedPatchesFolderPrefix)) {
throw JSONException("Diff manifest patchedFiles[\"$relativePath\"] field \"patch\" must be under the reserved \"$reservedPatchesFolderPrefix\" prefix, but is \"$patch\".")
}
PatchedFileEntry(
algo = entry.getString("algo"),
baseHash = entry.getString("baseHash"),
targetHash = entry.getString("targetHash"),
patch = entry.getString("patch"),
patch = patch,
)
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,36 @@ class BinaryDiffPatcherTest {
assertEquals(0, applier.invocationCount)
}

@Test
fun applyBinaryDiffPatches_patchFieldResolvesToUnzippedFolderItself_throwsWithoutInvokingApplier() {
// Given
val currentPackageFolder = tempFolder.newFolder("current")
val oldFile = File(currentPackageFolder, "index.android.bundle").apply { writeText("old hermes bytecode contents") }
val unzippedFolder = tempFolder.newFolder("unzipped")
val newUpdateFolder = tempFolder.newFolder("newUpdate")

val manifest = manifestOf(
mapOf(
"index.android.bundle" to PatchedFileEntry(
algo = "bsdiff",
baseHash = sha256Hex(oldFile),
targetHash = "irrelevant",
patch = "__hcp_patches/..",
)
)
)
val applier = FakePatchApplier { _, _, _ -> DiffPatch.PatchResult.OK }

// When / Then
try {
applyBinaryDiffPatches(manifest, currentPackageFolder, unzippedFolder, newUpdateFolder, applier)
fail("expected BinaryDiffApplyException")
} catch (e: BinaryDiffApplyException) {
assertEquals("__hcp_patches/..", e.relativePath)
}
assertEquals(0, applier.invocationCount)
}

@Test
fun applyBinaryDiffPatches_realBsdiffFixtureShape_appliesSuccessfully() {
fun fixture(name: String) =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.microsoft.codepush.react.diffpatch
import org.json.JSONObject
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
import org.junit.Assert.fail
import org.junit.Test

class DiffManifestTest {
Expand Down Expand Up @@ -124,6 +125,43 @@ class DiffManifestTest {
parseDiffManifest(json)
}

private fun assertPatchFieldRejectedByParser(patch: String) {
// Given
val json = JSONObject(
"""
{
"version": 2,
"patchedFiles": {
"relative/path.js": {
"algo": "bsdiff",
"baseHash": "base-hash-1",
"targetHash": "target-hash-1",
"patch": "$patch"
}
}
}
""".trimIndent()
)

// When / Then
try {
parseDiffManifest(json)
fail("expected JSONException")
} catch (e: org.json.JSONException) {
assertTrue(e.message, e.message!!.contains("__hcp_patches/"))
}
}

@Test
fun parseDiffManifest_patchedFileEntryPatchOutsideReservedPrefix_throws() {
assertPatchFieldRejectedByParser("relative/path.js.bsdiff")
}

@Test
fun parseDiffManifest_patchedFileEntryPatchWithPrefixButNoSlash_throws() {
assertPatchFieldRejectedByParser("__hcp_patchesX/relative/path.js.bsdiff")
}

@Test(expected = org.json.JSONException::class)
fun parseDiffManifest_patchedFileEntryMissingRequiredField_throws() {
// Given
Expand Down
3 changes: 3 additions & 0 deletions ios/CodePush/CodePushDiffManifest.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

NS_ASSUME_NONNULL_BEGIN

// Folder within the update ZIP that contains the diff patches. Must be in sync with server-side impl.
extern NSString *const CodePushDiffPatchesFolderName;

@interface CodePushPatchedFileEntry : NSObject

// The only value this client understands at the moment is "bsdiff".
Expand Down
11 changes: 10 additions & 1 deletion ios/CodePush/CodePushDiffManifest.m
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
return [CodePushErrorUtils errorWithMessage:[NSString stringWithFormat:@"Diff manifest %@ field \"%@\" must be a string, but is %@", context, fieldName, NSStringFromClass([value class])]];
}

NSString *const CodePushDiffPatchesFolderName = @"__hcp_patches";

static BOOL isAbsent(id value)
{
return value == nil || [value isKindOfClass:[NSNull class]];
Expand Down Expand Up @@ -170,10 +172,17 @@ + (nullable instancetype)manifestFromJSON:(NSDictionary *)json error:(NSError **
}
}

NSString *patch = entryJSON[@"patch"];
NSString *reservedPatchesFolderPrefix = [CodePushDiffPatchesFolderName stringByAppendingString:@"/"];
if (![patch hasPrefix:reservedPatchesFolderPrefix]) {
if (error) *error = [CodePushErrorUtils errorWithMessage:[NSString stringWithFormat:@"Diff manifest %@ field \"patch\" must be under the reserved \"%@\" prefix, but is \"%@\"", context, reservedPatchesFolderPrefix, patch]];
return nil;
}

patchedFiles[relativePath] = [[CodePushPatchedFileEntry alloc] initWithAlgo:entryJSON[@"algo"]
baseHash:entryJSON[@"baseHash"]
targetHash:entryJSON[@"targetHash"]
patch:entryJSON[@"patch"]];
patch:patch];
}
}

Expand Down
4 changes: 1 addition & 3 deletions ios/CodePush/CodePushPackage.m
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ @implementation CodePushPackage
#pragma mark - Private constants

static NSString *const DiffManifestFileName = @"hotcodepush.json";
// Folder within the update ZIP that contains the diff patches.
static NSString *const DiffPatchesFolderName = @"__hcp_patches";
static NSString *const DownloadFileName = @"download.zip";
static NSString *const RelativeBundlePathKey = @"bundlePath";
static NSString *const StatusFile = @"codepush.json";
Expand Down Expand Up @@ -78,7 +76,7 @@ + (BOOL)applyDiffManifest:(CodePushDiffManifest *)diffManifest
// The patches folder must not stay in the installed package: it is
// not part of the released contents, so it changes the folder hash
// and surfaces later as a misleading integrity-check failure.
NSString *patchesFolderPath = [newUpdateFolderPath stringByAppendingPathComponent:DiffPatchesFolderName];
NSString *patchesFolderPath = [newUpdateFolderPath stringByAppendingPathComponent:CodePushDiffPatchesFolderName];
if ([[NSFileManager defaultManager] fileExistsAtPath:patchesFolderPath]) {
NSError *removeError = nil;
BOOL patchesFolderRemoved = [[NSFileManager defaultManager] removeItemAtPath:patchesFolderPath
Expand Down
36 changes: 36 additions & 0 deletions ios/CodePushTests/CodePushDiffManifestTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,42 @@ final class CodePushDiffManifestTests: XCTestCase {
XCTAssertThrowsError(try CodePushDiffManifest(json: jsonWithPatchedFiles))
}

func testManifest_patchedFilesEntryPatchOutsideReservedPrefix_throws() {
let json: [AnyHashable: Any] = [
"version": 2,
"patchedFiles": [
"main.jsbundle": [
"algo": "bsdiff",
"baseHash": "aaaa",
"targetHash": "bbbb",
"patch": "main.jsbundle.bsdiff",
]
],
]

XCTAssertThrowsError(try CodePushDiffManifest(json: json)) { error in
XCTAssertTrue(error.localizedDescription.contains("__hcp_patches/"), error.localizedDescription)
}
}

func testManifest_patchedFilesEntryPatchWithPrefixButNoSlash_throws() {
let json: [AnyHashable: Any] = [
"version": 2,
"patchedFiles": [
"main.jsbundle": [
"algo": "bsdiff",
"baseHash": "aaaa",
"targetHash": "bbbb",
"patch": "__hcp_patchesX/main.jsbundle.bsdiff",
]
],
]

XCTAssertThrowsError(try CodePushDiffManifest(json: json)) { error in
XCTAssertTrue(error.localizedDescription.contains("__hcp_patches/"), error.localizedDescription)
}
}

func testManifest_patchedFilesWithoutVersionTwo_throws() {
let json: [AnyHashable: Any] = [
"version": 1,
Expand Down
Loading