Commit db12621f authored by Narayan Kamath's avatar Narayan Kamath
Browse files

StrictJarFile: Fix CheckJNI crashes due to invalid UTF-8 bytes.

Use the java.lang.String(byte[]) constructor instead of NewStringUTF
since the former replaces malformed and unmappable characters instead
of faulting on them. This also gives us some degree of consistency
when it comes to how these names are handled.

From StrictJarFile's perspective, the entry name is only used by the
JarVerifier for two things :

(1) keeping track of entries it has seen - this only requires the entry
name to be stable.

(2) looking up entry attributes in the manifest - this will continue
to work if the same bogus encoding is used in the manifest. If the
manifest uses valid UTF-8, the attribute lookup will fail and we'll
return no certificates for the entry (this is a good thing).

Finding an entry by name remains unaffacted because (as before) we
require byte by byte equality with the input.

The longer term fix is to disallow non-UTF entry names altogether but
that needs a bit more advance notice.

bug: 18584205
Change-Id: I7bb3e5bb09962d768a28aca4a6ece4dd54aa3473
parent 4cd98dfe
......@@ -32,7 +32,28 @@ static void throwIoException(JNIEnv* env, const int32_t errorCode) {
jniThrowException(env, "java/io/IOException", ErrorCodeString(errorCode));
}
static jobject newZipEntry(JNIEnv* env, const ZipEntry& entry, jstring entryName,
// Constructs a string out of |name| with the default charset (UTF-8 on android).
// We prefer this to JNI's NewStringUTF because the string constructor will
// replace unmappable and malformed bytes instead of throwing. See b/18584205
//
// Returns |NULL| iff. we couldn't allocate the string object or its constructor
// arguments.
//
// TODO: switch back to NewStringUTF after libziparchive is modified to reject
// files whose names aren't valid UTF-8.
static jobject constructString(JNIEnv* env, const char* name, const uint16_t nameLength) {
jbyteArray javaNameBytes = env->NewByteArray(nameLength);
if (javaNameBytes == NULL) {
return NULL;
}
env->SetByteArrayRegion(javaNameBytes, 0, nameLength, reinterpret_cast<const jbyte*>(name));
ScopedLocalRef<jclass> stringClass(env, env->FindClass("java/lang/String"));
const jmethodID stringCtor = env->GetMethodID(stringClass.get(), "<init>", "([B)V");
return env->NewObject(stringClass.get(), stringCtor, javaNameBytes);
}
static jobject newZipEntry(JNIEnv* env, const ZipEntry& entry, const jobject entryName,
const uint16_t nameLength) {
ScopedLocalRef<jclass> zipEntryClass(env, env->FindClass("java/util/zip/ZipEntry"));
const jmethodID zipEntryCtor = env->GetMethodID(zipEntryClass.get(), "<init>",
......@@ -54,6 +75,11 @@ static jobject newZipEntry(JNIEnv* env, const ZipEntry& entry, jstring entryName
static_cast<jlong>(entry.offset));
}
static jobject newZipEntry(JNIEnv* env, const ZipEntry& entry, const char* name,
const uint16_t nameLength) {
return newZipEntry(env, entry, constructString(env, name, nameLength), nameLength);
}
static jlong StrictJarFile_nativeOpenJarFile(JNIEnv* env, jobject, jstring fileName) {
ScopedUtfChars fileChars(env, fileName);
if (fileChars.c_str() == NULL) {
......@@ -133,9 +159,8 @@ static jobject StrictJarFile_nativeNextEntry(JNIEnv* env, jobject, jlong iterati
UniquePtr<char[]> entryNameCString(new char[entryName.name_length + 1]);
memcpy(entryNameCString.get(), entryName.name, entryName.name_length);
entryNameCString[entryName.name_length] = '\0';
ScopedLocalRef<jstring> entryNameString(env, env->NewStringUTF(entryNameCString.get()));
return newZipEntry(env, data, entryNameString.get(), entryName.name_length);
return newZipEntry(env, data, entryNameCString.get(), entryName.name_length);
}
static jobject StrictJarFile_nativeFindEntry(JNIEnv* env, jobject, jlong nativeHandle,
......@@ -169,5 +194,4 @@ static JNINativeMethod gMethods[] = {
void register_java_util_jar_StrictJarFile(JNIEnv* env) {
jniRegisterNativeMethods(env, "java/util/jar/StrictJarFile", gMethods, NELEM(gMethods));
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment