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
18 changes: 18 additions & 0 deletions ios/CodePush.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@
3F6A18E4C1B72D094F5E8A67 /* CodePushErrorUtils.m in Sources */ = {isa = PBXBuildFile; fileRef = 1B762E8F1C9A5E9A006EF800 /* CodePushErrorUtils.m */; };
C11A2DB29D3D814B1A7891DF /* CodePushDiffManifest.h in Headers */ = {isa = PBXBuildFile; fileRef = 2A092598DCE51279CA317823 /* CodePushDiffManifest.h */; };
4693408DCB9ACB8E02BF69B1 /* CodePushDiffManifest.h in Headers */ = {isa = PBXBuildFile; fileRef = 2A092598DCE51279CA317823 /* CodePushDiffManifest.h */; };
3643F3729205426163367671 /* CodePushBinaryDiffPatcherTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F42FA68BF21AED765F55E71A /* CodePushBinaryDiffPatcherTests.swift */; };
1993F82DF6330426AC5CBC2E /* CodePushBinaryDiffPatcher.m in Sources */ = {isa = PBXBuildFile; fileRef = 9414C770C70E1B1319F24B12 /* CodePushBinaryDiffPatcher.m */; };
F41E2EFAD0D322F7D14D7125 /* CodePushBinaryDiffPatcher.m in Sources */ = {isa = PBXBuildFile; fileRef = 9414C770C70E1B1319F24B12 /* CodePushBinaryDiffPatcher.m */; };
D5BC8B1BF7D2DA03BB888FCC /* CodePushBinaryDiffPatcher.m in Sources */ = {isa = PBXBuildFile; fileRef = 9414C770C70E1B1319F24B12 /* CodePushBinaryDiffPatcher.m */; };
Comment on lines +19 to +21

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These three entries add CodePushBinaryDiffPatcher.m to CodePush, CodePush-tvOS and CodePushTests — but shared/diffpatch/bspatch_bridge.c (and the hdiffpatch sources) are not in the tvOS target, and the patcher's codepush_bspatch_apply call at CodePushBinaryDiffPatcher.m:89 is its only caller. I parsed both trees to confirm:

BASE (#58)  tvOS: bspatch_bridge.c=no  patcher=no   Sha256=no  UpdateUtils=YES
PR#59       tvOS: bspatch_bridge.c=no  patcher=YES  Sha256=no  UpdateUtils=YES

So _codepush_bspatch_apply becomes unresolved in CodePush-tvOS, and it wasn't before this PR.

Two things keep this from being a build break, and I want to be precise about them rather than overstate it: CodePush-tvOS is a static library, so unresolved symbols are legal in the archive and xcodebuild will not fail on that target. And CocoaPods consumers are unaffected — the podspec compiles shared/diffpatch/*.c and the hdiffpatch sources for tvOS as well. It surfaces at app link time only for someone integrating tvOS manually from the .xcodeproj.

Separately and pre-existing, so not yours to fix here: _CodePushSha256HexForFile is also unresolved in that target — CodePushUpdateUtils.m already referenced it on the base branch with CodePushSha256.m absent. The patcher just becomes a second caller.

Either drop the patcher from CodePush-tvOS, or add bspatch_bridge.c + the hdiffpatch sources (and CodePushSha256.m) to it.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tvOS target frequently gets flagged, and we should clean it up one day, I just want to better understand what was its original purpose. It's been broken for years, we don't really use it in CI anyway, and it doesn't get distributed to users (Cocoapods is the distribution mechanism). I think it's fine for now.

E5C712D353D9164F04A2F456 /* CodePushBinaryDiffPatcher.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F588BE40F9B3092085C5B63 /* CodePushBinaryDiffPatcher.h */; };
469472C57F1D5FF7C0274268 /* CodePushBinaryDiffPatcher.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F588BE40F9B3092085C5B63 /* CodePushBinaryDiffPatcher.h */; };
0ABCB5DEFE01A7A15552A498 /* file_for_patch.c in Sources */ = {isa = PBXBuildFile; fileRef = 70779807AB59EA4711737F4E /* file_for_patch.c */; };
0EC08441DDDD08C96DDE851C /* CodePushSha256Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D23322E9E8EFBE01F1FACFB1 /* CodePushSha256Tests.swift */; };
13BE3DEE1AC21097009241FE /* CodePush.m in Sources */ = {isa = PBXBuildFile; fileRef = 13BE3DED1AC21097009241FE /* CodePush.m */; };
Expand Down Expand Up @@ -195,6 +201,9 @@
9645879798B46D35D8D824F8 /* CodePushDiffManifestTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = CodePushDiffManifestTests.swift; sourceTree = "<group>"; };
2A092598DCE51279CA317823 /* CodePushDiffManifest.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = CodePushDiffManifest.h; path = CodePush/CodePushDiffManifest.h; sourceTree = "<group>"; };
49166671F067D5F6429B262B /* CodePushDiffManifest.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = CodePushDiffManifest.m; path = CodePush/CodePushDiffManifest.m; sourceTree = "<group>"; };
F42FA68BF21AED765F55E71A /* CodePushBinaryDiffPatcherTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = CodePushBinaryDiffPatcherTests.swift; sourceTree = "<group>"; };
0F588BE40F9B3092085C5B63 /* CodePushBinaryDiffPatcher.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = CodePushBinaryDiffPatcher.h; path = CodePush/CodePushBinaryDiffPatcher.h; sourceTree = "<group>"; };
9414C770C70E1B1319F24B12 /* CodePushBinaryDiffPatcher.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = CodePushBinaryDiffPatcher.m; path = CodePush/CodePushBinaryDiffPatcher.m; sourceTree = "<group>"; };
0BF68F85125CF81D7EB65ABB /* bspatch_bridge.h */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.c.h; path = bspatch_bridge.h; sourceTree = "<group>"; };
0DC7989C75C72A774EF3685F /* bsdiff_wrapper/bspatch_wrapper.c */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.c.c; path = bsdiff_wrapper/bspatch_wrapper.c; sourceTree = "<group>"; };
134814201AA4EA6300B7C361 /* libCodePush.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = libCodePush.a; sourceTree = BUILT_PRODUCTS_DIR; };
Expand Down Expand Up @@ -451,6 +460,8 @@
810D4E6C1B96935000B397E9 /* CodePushPackage.m */,
2A092598DCE51279CA317823 /* CodePushDiffManifest.h */,
49166671F067D5F6429B262B /* CodePushDiffManifest.m */,
0F588BE40F9B3092085C5B63 /* CodePushBinaryDiffPatcher.h */,
9414C770C70E1B1319F24B12 /* CodePushBinaryDiffPatcher.m */,
5421FE301C58AD5A00986A55 /* CodePushTelemetryManager.m */,
540D20111C7684FE00D6EF41 /* CodePushUpdateUtils.m */,
1B23B9131BF9267B000BB2F0 /* RCTConvert+CodePushInstallMode.m */,
Expand Down Expand Up @@ -483,6 +494,7 @@
children = (
95D6DD0EACAC8D095880DFD0 /* BSPatchTests.swift */,
9645879798B46D35D8D824F8 /* CodePushDiffManifestTests.swift */,
F42FA68BF21AED765F55E71A /* CodePushBinaryDiffPatcherTests.swift */,
E9FA144425AE78B97AD6C870 /* CodePushTests-Bridging-Header.h */,
E74D80ECD8DD03C4C0772B7B /* Fixtures */,
D23322E9E8EFBE01F1FACFB1 /* CodePushSha256Tests.swift */,
Expand Down Expand Up @@ -682,6 +694,7 @@
3221E4642C8ABE1300268379 /* mz_strm_split.h in Headers */,
6463C8471EBA0D290095B8CD /* CodePush.h in Headers */,
4693408DCB9ACB8E02BF69B1 /* CodePushDiffManifest.h in Headers */,
469472C57F1D5FF7C0274268 /* CodePushBinaryDiffPatcher.h in Headers */,
3221E46A2C8ABE1300268379 /* mz_strm.h in Headers */,
3221E4782C8ABE1300268379 /* mz_zip_rw.h in Headers */,
3221E4802C8ABE1400268379 /* mz.h in Headers */,
Expand Down Expand Up @@ -743,6 +756,7 @@
3221E4752C8ABE1300268379 /* mz_strm_pkcrypt.h in Headers */,
8482F84E1E24C66300F793DB /* CodePush.h in Headers */,
C11A2DB29D3D814B1A7891DF /* CodePushDiffManifest.h in Headers */,
E5C712D353D9164F04A2F456 /* CodePushBinaryDiffPatcher.h in Headers */,
F88664711F4AD1EE0036D01B /* JWTCoding+VersionTwo.h in Headers */,
F886646B1F4AD1EE0036D01B /* JWTCoding+ResultTypes.h in Headers */,
3221E4652C8ABE1300268379 /* mz_strm_buf.h in Headers */,
Expand Down Expand Up @@ -913,6 +927,7 @@
3221E4612C8ABE1300268379 /* mz_zip_rw.c in Sources */,
810D4E6D1B96935000B397E9 /* CodePushPackage.m in Sources */,
CED0A6936F6F8C354302F246 /* CodePushDiffManifest.m in Sources */,
1993F82DF6330426AC5CBC2E /* CodePushBinaryDiffPatcher.m in Sources */,
3221E4552C8ABE1300268379 /* mz_strm_pkcrypt.c in Sources */,
F88664531F4AD1EE0036D01B /* JWTAlgorithmESBase.m in Sources */,
3221E4532C8ABE1300268379 /* mz_strm_os_posix.c in Sources */,
Expand All @@ -938,6 +953,7 @@
3221E46E2C8ABE1300268379 /* mz_crypt.c in Sources */,
6463C8321EBA0CFB0095B8CD /* CodePushPackage.m in Sources */,
CD32522E04D0F0E86DCC7BBE /* CodePushDiffManifest.m in Sources */,
F41E2EFAD0D322F7D14D7125 /* CodePushBinaryDiffPatcher.m in Sources */,
6463C8331EBA0CFB0095B8CD /* CodePushTelemetryManager.m in Sources */,
6463C8341EBA0CFB0095B8CD /* CodePushUpdateUtils.m in Sources */,
3221E45C2C8ABE1300268379 /* mz_strm_zlib.c in Sources */,
Expand All @@ -961,9 +977,11 @@
files = (
B5B50F91FAAF80444988E284 /* BSPatchTests.swift in Sources */,
BBC7F97A68E454FB38953FC4 /* CodePushDiffManifestTests.swift in Sources */,
3643F3729205426163367671 /* CodePushBinaryDiffPatcherTests.swift in Sources */,
47F66D5AF3C3185E1A3E3B15 /* bspatch_bridge.c in Sources */,
0E9314F338AC505D2E33C1A3 /* CodePushDiffManifest.m in Sources */,
3F6A18E4C1B72D094F5E8A67 /* CodePushErrorUtils.m in Sources */,
D5BC8B1BF7D2DA03BB888FCC /* CodePushBinaryDiffPatcher.m in Sources */,
DC983F1C71E0E7131BB343C5 /* libHDiffPatch/HPatch/patch.c in Sources */,
A88F11124A2120A8373A8B61 /* bsdiff_wrapper/bspatch_wrapper.c in Sources */,
0ABCB5DEFE01A7A15552A498 /* file_for_patch.c in Sources */,
Expand Down
25 changes: 25 additions & 0 deletions ios/CodePush/CodePushBinaryDiffPatcher.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#import <Foundation/Foundation.h>
#import "CodePushDiffManifest.h"

NS_ASSUME_NONNULL_BEGIN

@interface CodePushBinaryDiffPatcher : NSObject

// Applies every entry in manifest.patchedFiles: verifies the pre-patch file
// against baseHash, applies the patch into newUpdateFolder, then verifies
// the result against targetHash.
//
// All three folder arguments must exist on disk. Files within them need not,
// the patch output and its parent directories are created as needed.
//
// Returns NO and sets *error on the first failure.
+ (BOOL)applyBinaryDiffPatchesFromManifest:(CodePushDiffManifest *)manifest
currentPackageFolder:(NSString *)currentPackageFolder
unzippedFolder:(NSString *)unzippedFolder
newUpdateFolder:(NSString *)newUpdateFolder
error:(NSError **)error
NS_SWIFT_NAME(applyBinaryDiffPatches(manifest:currentPackageFolder:unzippedFolder:newUpdateFolder:));

@end

NS_ASSUME_NONNULL_END
113 changes: 113 additions & 0 deletions ios/CodePush/CodePushBinaryDiffPatcher.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
#import "CodePushBinaryDiffPatcher.h"
#import "CodePushErrorUtils.h"
#import "CodePushSha256.h"
#import "bspatch_bridge.h"

static NSError *patchApplyError(NSString *relativePath, NSString *reason)
{
return [CodePushErrorUtils errorWithMessage:
[NSString stringWithFormat:@"Failed to apply binary diff patch for \"%@\": %@", relativePath, reason]];
}

// Keep in sync with shared/diffpatch/bspatch_bridge.h.
static NSString *describeBSPatchResult(CodePushBSPatchResult result)
{
switch (result) {
case CODEPUSH_BSPATCH_OK: return @"OK";
case CODEPUSH_BSPATCH_ERR_BAD_DIFF_HEADER: return @"BAD_DIFF_HEADER";
case CODEPUSH_BSPATCH_ERR_OPEN_OLD: return @"OPEN_OLD_FAILED";
case CODEPUSH_BSPATCH_ERR_OPEN_DIFF: return @"OPEN_DIFF_FAILED";
case CODEPUSH_BSPATCH_ERR_OPEN_OUT: return @"OPEN_OUT_FAILED";
case CODEPUSH_BSPATCH_ERR_OOM: return @"OUT_OF_MEMORY";
case CODEPUSH_BSPATCH_ERR_PATCH_FAILED: return @"PATCH_FAILED";
}
return [NSString stringWithFormat:@"UNKNOWN (%ld)", (long)result];
}

static NSString *resolveWithin(NSString *base, NSString *path, NSString *manifestEntry, NSError **error)
{
NSError *resolveError = nil;
NSString *resolved = [CodePushDiffManifest resolvePath:path withinFolder:base error:&resolveError];
if (resolved == nil && error) {
*error = patchApplyError(manifestEntry, resolveError.localizedDescription);
}
return resolved;
}
Comment on lines +27 to +35

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This maps every nil from resolvePath:withinFolder: to "path escapes expected directory", but #58's resolver returns nil for at least four distinct causes:

Two of those are manifest-triggerable: patchedFiles: {"": {...}}, or a key containing a NUL, both report a path escape for what is only a malformed manifest.

Worth noting this is an iOS-only divergence. Android's resolveWithin uses File.canonicalFile, which doesn't require the path to exist, so a missing currentPackageFolder there surfaces as a genuine read failure out of sha256Hex instead. Same manifest, same condition, very different diagnosis — and "path escapes" is the one that sends whoever reads the log looking for an attack.

Threading the reason out of resolvePath (or at least distinguishing "base folder unusable" and "path not representable" from a real escape) would fix it.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Decided to fix this by adopting the standard ObjC pattern of a NSError* argument. The caller can log this root cause, and ObjC -> Swift interop transforms this pattern into real exceptions.

PR #58 is updated with the change to resolvePath:withinFolder, and this PR with the changes to the callsites.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good choice, thanks!


@implementation CodePushBinaryDiffPatcher

+ (BOOL)applyBinaryDiffPatchesFromManifest:(CodePushDiffManifest *)manifest
currentPackageFolder:(NSString *)currentPackageFolder
unzippedFolder:(NSString *)unzippedFolder
newUpdateFolder:(NSString *)newUpdateFolder
error:(NSError **)error
{
NSDictionary<NSString *, CodePushPatchedFileEntry *> *patchedFiles = manifest.patchedFiles;

for (NSString *relativePath in patchedFiles) {
CodePushPatchedFileEntry *entry = patchedFiles[relativePath];
if (![entry.algo isEqualToString:@"bsdiff"]) {
if (error) *error = patchApplyError(relativePath, [NSString stringWithFormat:@"unsupported patch algorithm: %@", entry.algo]);
return NO;
}
}

NSFileManager *fileManager = [NSFileManager defaultManager];

for (NSString *relativePath in patchedFiles) {
CodePushPatchedFileEntry *entry = patchedFiles[relativePath];

NSString *oldFile = resolveWithin(currentPackageFolder, relativePath, relativePath, error);
if (!oldFile) return NO;

NSError *oldFileHashError = nil;
NSString *oldFileHash = CodePushSha256HexForFile(oldFile, &oldFileHashError);
if (!oldFileHash) {
if (error) *error = patchApplyError(relativePath, [NSString stringWithFormat:@"could not hash the file to patch: %@", oldFileHashError.localizedDescription]);
return NO;
}
if (![oldFileHash isEqualToString:entry.baseHash]) {
if (error) *error = patchApplyError(relativePath, [NSString stringWithFormat:@"baseHash mismatch: expected %@, got %@", entry.baseHash, oldFileHash]);
return NO;
}

NSString *diffFile = resolveWithin(unzippedFolder, entry.patch, relativePath, error);
if (!diffFile) return NO;

NSString *newFile = resolveWithin(newUpdateFolder, relativePath, relativePath, error);
if (!newFile) return NO;

NSError *createDirError = nil;
[fileManager createDirectoryAtPath:[newFile stringByDeletingLastPathComponent]
withIntermediateDirectories:YES
attributes:nil
error:&createDirError];
if (createDirError) {
if (error) *error = patchApplyError(relativePath, createDirError.localizedDescription);
return NO;
}

CodePushBSPatchResult result = codepush_bspatch_apply(oldFile.fileSystemRepresentation,
diffFile.fileSystemRepresentation,
newFile.fileSystemRepresentation);
if (result != CODEPUSH_BSPATCH_OK) {
if (error) *error = patchApplyError(relativePath, [NSString stringWithFormat:@"patch failed: %@", describeBSPatchResult(result)]);
return NO;
}

NSError *newFileHashError = nil;
NSString *newFileHash = CodePushSha256HexForFile(newFile, &newFileHashError);
if (!newFileHash) {
if (error) *error = patchApplyError(relativePath, [NSString stringWithFormat:@"could not hash the patched file: %@", newFileHashError.localizedDescription]);
return NO;
}
if (![newFileHash isEqualToString:entry.targetHash]) {
if (error) *error = patchApplyError(relativePath, [NSString stringWithFormat:@"targetHash mismatch: expected %@, got %@", entry.targetHash, newFileHash]);
return NO;
}
}

return YES;
}

@end
Loading
Loading