diff --git a/app/src/main/java/com/httrack/android/HTTrackActivity.java b/app/src/main/java/com/httrack/android/HTTrackActivity.java index cf8aef5..9495a45 100755 --- a/app/src/main/java/com/httrack/android/HTTrackActivity.java +++ b/app/src/main/java/com/httrack/android/HTTrackActivity.java @@ -802,6 +802,11 @@ protected String[] getProjectNames() { protected File getTargetFile() { final String name = mapper.getProjectName(); if (name != null && name.length() != 0) { + // Backstop: a name that is not a single in-root segment must never become an -O path. + if (!StoragePaths.isValidProjectName(name)) { + Log.w(getClass().getSimpleName(), "rejecting unsafe project name: " + name); + return null; + } return new File(getProjectRootFile(), name); } else { return null; @@ -2155,6 +2160,11 @@ protected boolean validatePane() { dirtyNamePane = false; } } + // A crafted winprofile.ini can overwrite the name with a path that escapes the root. + if (!StoragePaths.isValidProjectName(mapper.getProjectName())) { + showNotification(getString(R.string.invalid_project_name), true); + return false; + } return true; } return false; diff --git a/app/src/main/java/com/httrack/android/StoragePaths.java b/app/src/main/java/com/httrack/android/StoragePaths.java index fc24f5a..3d61b70 100644 --- a/app/src/main/java/com/httrack/android/StoragePaths.java +++ b/app/src/main/java/com/httrack/android/StoragePaths.java @@ -40,4 +40,26 @@ static Boolean isWritable(final File path, final File external, final File inter } return external != null ? Boolean.FALSE : null; } + + /** + * Whether {@code name} is safe to append to a root as one directory. File does not fold "..", so + * a crafted winprofile.ini name like "../../sdcard/evil" would otherwise steer the engine's -O + * outside the Websites tree. Rejects empty, ".", ".." and anything carrying a path separator. + * + * @param name + * the project name, already whitespace-collapsed + * @return true if it denotes a single, in-root directory segment + */ + static boolean isValidProjectName(final String name) { + if (name == null || name.isEmpty() || name.equals(".") || name.equals("..")) { + return false; + } + for (int i = 0; i < name.length(); i++) { + final char c = name.charAt(i); + if (c == '/' || c == '\\' || c == '\0') { + return false; + } + } + return true; + } } diff --git a/app/src/main/jni/htslibjni-private.h b/app/src/main/jni/htslibjni-private.h index 6edfa55..6f67c38 100644 --- a/app/src/main/jni/htslibjni-private.h +++ b/app/src/main/jni/htslibjni-private.h @@ -137,16 +137,56 @@ typedef struct hts_state_t { const char *message; } hts_state_t; -/* NewStringUTF, but ignore invalid UTF-8 or NULL input. */ +/* Sanitize server-controlled bytes to well-formed modified UTF-8 (invalid sequences, including + 4-byte forms, become '?'), so hostile input cannot reach NewStringUTF, which aborts under + CheckJNI. Caller frees the result. */ +static char *sanitizeModifiedUtf8(const char *s) { + const size_t len = strlen(s); + char *const out = malloc(len + 1); + if (out != NULL) { + const unsigned char *const in = (const unsigned char *) s; + size_t i = 0, j = 0; + while (i < len) { + const unsigned char c = in[i]; + unsigned int seq = 0; + if (c <= 0x7F) { + seq = 1; + } else if (c >= 0xC2 && c <= 0xDF) { + if (i + 1 < len && (in[i + 1] & 0xC0) == 0x80) { + seq = 2; + } + } else if (c >= 0xE0 && c <= 0xEF) { + /* Reject overlong E0 80..9F; surrogate halves (ED A0..BF) stay, they are valid here. */ + const unsigned char lo = (c == 0xE0) ? 0xA0 : 0x80; + if (i + 2 < len && in[i + 1] >= lo && (in[i + 1] & 0xC0) == 0x80 + && (in[i + 2] & 0xC0) == 0x80) { + seq = 3; + } + } + if (seq != 0) { + for (unsigned int k = 0; k < seq; k++) { + out[j++] = (char) in[i++]; + } + } else { + out[j++] = '?'; + i++; + } + } + out[j] = '\0'; + } + return out; +} + +/* NewStringUTF over sanitized bytes, so hostile input cannot reach it; NULL input yields NULL. */ static jobject newStringSafe(JNIEnv *env, const char *s) { if (s != NULL) { - const int ne = ! (*env)->ExceptionOccurred(env); - jobject str = (*env)->NewStringUTF(env, s); - /* Silently ignore UTF-8 exception. */ - if (str == NULL && (*env)->ExceptionOccurred(env) && ne) { - (*env)->ExceptionClear(env); + char *const safe = sanitizeModifiedUtf8(s); + /* On OOM, a null field beats feeding raw (possibly hostile) bytes to NewStringUTF. */ + if (safe != NULL) { + jobject str = (*env)->NewStringUTF(env, safe); + free(safe); + return str; } - return str; } return NULL; } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index e7085c9..e5cd905 100755 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -185,6 +185,7 @@ Could not write to: Read-only media (SDCARD) No storage media (SDCARD) + Invalid project name: it must not contain \'/\', \'\\\' or \'..\' HTTrack may not be able to download websites until this problem is fixed HTTrack: mirror \'%s\' stopped! Click on this notification to restart the interrupted mirror diff --git a/app/src/test/java/com/httrack/android/ProjectNameTest.java b/app/src/test/java/com/httrack/android/ProjectNameTest.java new file mode 100644 index 0000000..890ab8b --- /dev/null +++ b/app/src/test/java/com/httrack/android/ProjectNameTest.java @@ -0,0 +1,32 @@ +package com.httrack.android; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +/** Attacks StoragePaths.isValidProjectName: a name is one in-root directory segment, nothing more. */ +public class ProjectNameTest { + @Test + public void acceptsAPlainName() { + assertTrue(StoragePaths.isValidProjectName("My Website")); + assertTrue(StoragePaths.isValidProjectName("..dotdot..name")); + assertTrue(StoragePaths.isValidProjectName("a.b.c")); + } + + @Test + public void refusesEmptyOrDotSegments() { + assertFalse(StoragePaths.isValidProjectName(null)); + assertFalse(StoragePaths.isValidProjectName("")); + assertFalse(StoragePaths.isValidProjectName(".")); + assertFalse(StoragePaths.isValidProjectName("..")); + } + + @Test + public void refusesAnyPathSeparator() { + assertFalse(StoragePaths.isValidProjectName("../../../../sdcard/Android/data/evil")); + assertFalse(StoragePaths.isValidProjectName("a/b")); + assertFalse(StoragePaths.isValidProjectName("a\\b")); + assertFalse(StoragePaths.isValidProjectName("evil\0hidden")); + } +}