Skip to content
Draft
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
93 changes: 43 additions & 50 deletions src/lib/installPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export default async function installPlugin(
let pluginDir;
let pluginUrl;
let state;
let tempZipFile;

try {
if (!(await fsOperation(PLUGIN_DIR).exists())) {
Expand Down Expand Up @@ -174,65 +175,48 @@ export default async function installPlugin(

// Track unsafe absolute entries to skip
const ignoredUnsafeEntries = new Set();

const files = Object.keys(zip.files);
const limit = 2;

async function processFile(file) {
try {
const entry = zip.files[file];

let correctFile = file.replace(/\\/g, "/");
const isDirEntry = entry.dir || correctFile.endsWith("/");

if (isUnsafeAbsolutePath(file)) {
ignoredUnsafeEntries.add(file);
return;
}

correctFile = sanitizeZipPath(correctFile, isDirEntry);
if (!correctFile) return;

const fileUrl = Url.join(pluginDir, correctFile);

// Handle directory entries
if (isDirEntry) {
await createFileRecursive(pluginDir, correctFile, true);
return;
}

// Ensure parent directory exists
const lastSlash = correctFile.lastIndexOf("/");
if (lastSlash !== -1) {
const parentRel = correctFile.slice(0, lastSlash + 1);
await createFileRecursive(pluginDir, parentRel, true);
}

if (!state.exists(correctFile)) {
await createFileRecursive(pluginDir, correctFile, false);
}
const tempZipName = `${id.replace(/[^a-zA-Z0-9]/g, "_")}.zip`;
tempZipFile = Url.join(CACHE_STORAGE, tempZipName);
if (await fsOperation(tempZipFile).exists()) {
await fsOperation(tempZipFile).delete();
}
await fsOperation(CACHE_STORAGE).createFile(tempZipName, plugin);

let data = await entry.async("ArrayBuffer");
// Natively unzip to plugin directory
await new Promise((resolve, reject) => {
system.unzip(tempZipFile, pluginDir, resolve, reject);
});

if (file === "plugin.json") {
data = JSON.stringify(pluginJson);
}
// Overwrite the original plugin.json inside pluginDir with the patched pluginJson
const pluginJsonFile = Url.join(pluginDir, "plugin.json");
if (await fsOperation(pluginJsonFile).exists()) {
await fsOperation(pluginJsonFile).writeFile(JSON.stringify(pluginJson));
} else {
await fsOperation(pluginDir).createFile(
"plugin.json",
JSON.stringify(pluginJson),
);
}

if (!(await state.isUpdated(correctFile, data))) return;
// Populate install state (updatedStore) and track skipped unsafe entries
for (const file of files) {
const entry = zip.files[file];
let correctFile = file.replace(/\\/g, "/");
const isDirEntry = entry.dir || correctFile.endsWith("/");

await fsOperation(fileUrl).writeFile(data);
} catch (error) {
console.error(`Error processing file ${file}:`, error);
if (isUnsafeAbsolutePath(file)) {
ignoredUnsafeEntries.add(file);
continue;
}
}

// Process in batches
for (let i = 0; i < files.length; i += limit) {
const batch = files.slice(i, i + limit);
await Promise.allSettled(batch.map(processFile));
correctFile = sanitizeZipPath(correctFile, isDirEntry);
if (!correctFile) continue;

// Allow UI thread to breathe
await new Promise((r) => setTimeout(r, 0));
if (!isDirEntry) {
state.updatedStore[correctFile.toLowerCase()] = "1";
}
}
Comment on lines +204 to 220

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.

P2 Install-state values changed from content hashes to a constant "1"

The old code stored a SHA-256 content hash in updatedStore via state.isUpdated(). The new loop writes "1" for every non-directory entry. After state.save() this persists to disk, so any future code path that reads the stored value expecting a real hash (e.g. a delta-update check in a future install) will instead see "1" and treat everything as changed. The existing state.exists() path works fine since it only tests key presence, but the semantics of the state file have silently shifted from "hash of last-installed content" to "was ever installed".

// Emit a non-blocking warning if any unsafe entries were skipped
if (!isDependency && ignoredUnsafeEntries.size) {
Expand Down Expand Up @@ -276,6 +260,15 @@ export default async function installPlugin(
}
throw err;
} finally {
if (tempZipFile) {
try {
if (await fsOperation(tempZipFile).exists()) {
await fsOperation(tempZipFile).delete();
}
} catch (e) {
console.error("Failed to delete tempZipFile:", e);
}
}
if (!isDependency) {
loaderDialog.destroy();
}
Expand Down
100 changes: 100 additions & 0 deletions src/plugins/system/android/com/foxdebug/system/System.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
import java.nio.file.Paths;
import java.nio.file.StandardOpenOption;
import java.io.IOException;
import java.util.zip.ZipInputStream;
import java.util.zip.ZipEntry;
import java.io.BufferedOutputStream;
import android.app.Activity;
import android.app.PendingIntent;
import android.content.ClipData;
Expand Down Expand Up @@ -214,6 +217,7 @@ public boolean execute(
case "compare-file-text":
case "compare-texts":
case "extractAsset":
case "unzip":
case "pin-file-shortcut":
break;
case "get-configuration":
Expand Down Expand Up @@ -442,6 +446,16 @@ public void run() {

}
return;

case "unzip":
try {
String zipPath = args.getString(0);
String destPath = args.getString(1);
unzip(zipPath, destPath, callbackContext);
} catch (Exception e) {
callbackContext.error("Failed to unzip: " + e.getMessage());
}
return;

case "getInstaller":
try {
Expand Down Expand Up @@ -2184,4 +2198,90 @@ private void extractAsset(String assetName, String destinationPath, CallbackCont
callback.error(sw.toString());
}
}

private void unzip(String zipPath, String destPath, CallbackContext callback) {
try {
Uri zipUri = Uri.parse(zipPath);
File destDir = null;
Uri destUri = Uri.parse(destPath);
if ("file".equalsIgnoreCase(destUri.getScheme())) {
destDir = new File(destUri.getPath());
} else {
destDir = new File(destPath);
}

if (!destDir.exists()) {
destDir.mkdirs();
}

String canonicalDestDirPath = destDir.getCanonicalPath();

InputStream is = null;
if ("file".equalsIgnoreCase(zipUri.getScheme())) {
is = new FileInputStream(new File(zipUri.getPath()));
} else if (zipUri.getScheme() != null) {
is = context.getContentResolver().openInputStream(zipUri);
} else {
is = new FileInputStream(new File(zipPath));
}

if (is == null) {
callback.error("Could not open input stream for zip file");
return;
}

try (ZipInputStream zis = new ZipInputStream(is)) {
ZipEntry entry;
byte[] buffer = new byte[8192];
while ((entry = zis.getNextEntry()) != null) {
String name = entry.getName();

// Normalize separator
name = name.replace('\\', '/');

// Skip unsafe absolute paths
if (name.startsWith("/") || name.contains("://") || name.startsWith("\\\\") || (name.length() > 1 && name.charAt(1) == ':')) {

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.

P2 Dead check: startsWith("\\\\") is unreachable

name has already been fully normalised by name.replace('\\', '/') on the line above, so no backslashes can remain. The name.startsWith("\\\\") branch will never be true. A UNC path like \\server\share becomes //server/share after the replace and is already blocked by the earlier name.startsWith("/") guard, so the protection still holds — but the second check is dead code and can mislead readers into thinking it provides independent coverage.

// Unsafe, skip it
zis.closeEntry();
continue;
}

// Resolve relative paths
File file = new File(destDir, name);
String canonicalFilePath = file.getCanonicalPath();

// Prevent Zip Slip
if (!canonicalFilePath.startsWith(canonicalDestDirPath + File.separator) && !canonicalFilePath.equals(canonicalDestDirPath)) {
zis.closeEntry();
continue;
}

if (entry.isDirectory()) {
file.mkdirs();
} else {
// Ensure parent directory exists
File parent = file.getParentFile();
if (parent != null && !parent.exists()) {
parent.mkdirs();
}

try (FileOutputStream fos = new FileOutputStream(file);
BufferedOutputStream bos = new BufferedOutputStream(fos, buffer.length)) {
int len;
while ((len = zis.read(buffer)) != -1) {
bos.write(buffer, 0, len);
}
bos.flush();
}
}
zis.closeEntry();
}
}
callback.success();
} catch (Exception e) {
StringWriter sw = new StringWriter();
e.printStackTrace(new PrintWriter(sw));
callback.error(sw.toString());
}
}
}
13 changes: 13 additions & 0 deletions src/plugins/system/system.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,19 @@ interface System {
onSuccess?: () => void,
onFail?: OnFail,
): void;
/**
* Unzip a zip archive natively
* @param zipFile Path or URI to the zip file
* @param destDir Target directory path to extract contents
* @param onSuccess Success callback
* @param onFail Error callback
*/
unzip(
zipFile: string,
destDir: string,
onSuccess: () => void,
onFail: OnFail,
): void;
}

interface Window{
Expand Down
3 changes: 3 additions & 0 deletions src/plugins/system/www/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -272,5 +272,8 @@ module.exports = {
[text1, text2]
);
});
},
unzip: function (zipFile, destDir, success, error) {
cordova.exec(success, error, 'System', 'unzip', [zipFile, destDir]);
}
};