Throw exception on odd length Signatures
The old version of this code would silently truncate odd-length
Signatures. However, this masks some bugs. Add a throw of
IllegalArgumentException so users can easily see where they're getting
bad input for Signatures.
Also, go through the existing code and catch this exception or
pre-check the input strings so system_server doesn't crash later.
Bug: 5092338
Change-Id: I8c672c5eaeb738a92c4581ce0df09baf719980ef
diff --git a/core/java/android/content/pm/Signature.java b/core/java/android/content/pm/Signature.java
index b32664e..c6aefb8 100644
--- a/core/java/android/content/pm/Signature.java
+++ b/core/java/android/content/pm/Signature.java
@@ -16,7 +16,6 @@
package android.content.pm;
-import android.content.ComponentName;
import android.os.Parcel;
import android.os.Parcelable;
@@ -40,26 +39,41 @@
mSignature = signature.clone();
}
+ private static final int parseHexDigit(int nibble) {
+ if ('0' <= nibble && nibble <= '9') {
+ return nibble - '0';
+ } else if ('a' <= nibble && nibble <= 'f') {
+ return nibble - 'a' + 10;
+ } else if ('A' <= nibble && nibble <= 'F') {
+ return nibble - 'A' + 10;
+ } else {
+ throw new IllegalArgumentException("Invalid character " + nibble + " in hex string");
+ }
+ }
+
/**
* Create Signature from a text representation previously returned by
- * {@link #toChars} or {@link #toCharsString()}.
+ * {@link #toChars} or {@link #toCharsString()}. Signatures are expected to
+ * be a hex-encoded ASCII string.
+ *
+ * @param text hex-encoded string representing the signature
+ * @throws IllegalArgumentException when signature is odd-length
*/
public Signature(String text) {
final byte[] input = text.getBytes();
final int N = input.length;
+
+ if (N % 2 != 0) {
+ throw new IllegalArgumentException("text size " + N + " is not even");
+ }
+
final byte[] sig = new byte[N / 2];
int sigIndex = 0;
for (int i = 0; i < N;) {
- int b;
-
- final int hi = input[i++];
- b = (hi >= 'a' ? (hi - 'a' + 10) : (hi - '0')) << 4;
-
- final int lo = input[i++];
- b |= (lo >= 'a' ? (lo - 'a' + 10) : (lo - '0')) & 0x0F;
-
- sig[sigIndex++] = (byte) (b & 0xFF);
+ final int hi = parseHexDigit(input[i++]);
+ final int lo = parseHexDigit(input[i++]);
+ sig[sigIndex++] = (byte) ((hi << 4) | lo);
}
mSignature = sig;
@@ -100,8 +114,7 @@
}
/**
- * Return the result of {@link #toChars()} as a String. This result is
- * cached so future calls will return the same String.
+ * Return the result of {@link #toChars()} as a String.
*/
public String toCharsString() {
String str = mStringRef == null ? null : mStringRef.get();
@@ -127,7 +140,7 @@
try {
if (obj != null) {
Signature other = (Signature)obj;
- return Arrays.equals(mSignature, other.mSignature);
+ return this == other || Arrays.equals(mSignature, other.mSignature);
}
} catch (ClassCastException e) {
}
diff --git a/services/java/com/android/server/BackupManagerService.java b/services/java/com/android/server/BackupManagerService.java
index 2e3d6dd..b489e33 100644
--- a/services/java/com/android/server/BackupManagerService.java
+++ b/services/java/com/android/server/BackupManagerService.java
@@ -3298,6 +3298,8 @@
}
} catch (NumberFormatException e) {
Slog.w(TAG, "Corrupt restore manifest for package " + info.packageName);
+ } catch (IllegalArgumentException e) {
+ Slog.w(TAG, e.getMessage());
}
return policy;
diff --git a/services/java/com/android/server/pm/PackageSignatures.java b/services/java/com/android/server/pm/PackageSignatures.java
index a25ec6c..9a20be7 100644
--- a/services/java/com/android/server/pm/PackageSignatures.java
+++ b/services/java/com/android/server/pm/PackageSignatures.java
@@ -138,6 +138,12 @@
"Error in package manager settings: <cert> "
+ "index " + index + " is not a number at "
+ parser.getPositionDescription());
+ } catch (IllegalArgumentException e) {
+ PackageManagerService.reportSettingsProblem(Log.WARN,
+ "Error in package manager settings: <cert> "
+ + "index " + index + " has an invalid signature at "
+ + parser.getPositionDescription() + ": "
+ + e.getMessage());
}
} else {
PackageManagerService.reportSettingsProblem(Log.WARN,