validate all data in repo push requests
This should get us closer towards not having to trust the server. fdroid/fdroidclient#1588 https://stackoverflow.com/questions/5205339/regular-expression-matching-fully-qualified-class-names/5205467
This commit is contained in:
parent
9a04ce4332
commit
9c8cc20a80
@ -344,8 +344,7 @@ public class RepoXMLHandler extends DefaultHandler {
|
|||||||
repoDescription = cleanWhiteSpace(attributes.getValue("", "description"));
|
repoDescription = cleanWhiteSpace(attributes.getValue("", "description"));
|
||||||
repoTimestamp = parseLong(attributes.getValue("", "timestamp"), 0);
|
repoTimestamp = parseLong(attributes.getValue("", "timestamp"), 0);
|
||||||
repoIcon = attributes.getValue("", "icon");
|
repoIcon = attributes.getValue("", "icon");
|
||||||
} else if (RepoPushRequest.INSTALL.equals(localName)
|
} else if (RepoPushRequest.VALID_REQUESTS.contains(localName)) {
|
||||||
|| RepoPushRequest.UNINSTALL.equals(localName)) {
|
|
||||||
if (repo.pushRequests == Repo.PUSH_REQUEST_ACCEPT_ALWAYS) {
|
if (repo.pushRequests == Repo.PUSH_REQUEST_ACCEPT_ALWAYS) {
|
||||||
RepoPushRequest r = new RepoPushRequest(
|
RepoPushRequest r = new RepoPushRequest(
|
||||||
localName,
|
localName,
|
||||||
|
@ -76,6 +76,7 @@ import java.util.List;
|
|||||||
import java.util.Locale;
|
import java.util.Locale;
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
import java.util.concurrent.TimeUnit;
|
import java.util.concurrent.TimeUnit;
|
||||||
|
import java.util.regex.Pattern;
|
||||||
|
|
||||||
public final class Utils {
|
public final class Utils {
|
||||||
|
|
||||||
@ -98,6 +99,8 @@ public final class Utils {
|
|||||||
private static DisplayImageOptions.Builder defaultDisplayImageOptionsBuilder;
|
private static DisplayImageOptions.Builder defaultDisplayImageOptionsBuilder;
|
||||||
private static DisplayImageOptions repoAppDisplayImageOptions;
|
private static DisplayImageOptions repoAppDisplayImageOptions;
|
||||||
|
|
||||||
|
private static Pattern safePackageNamePattern;
|
||||||
|
|
||||||
public static final String FALLBACK_ICONS_DIR = "/icons/";
|
public static final String FALLBACK_ICONS_DIR = "/icons/";
|
||||||
|
|
||||||
/*
|
/*
|
||||||
@ -621,6 +624,21 @@ public final class Utils {
|
|||||||
return sb;
|
return sb;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* This is not strict validation of the package name, this is just to make
|
||||||
|
* sure that the package name is not used as an attack vector, e.g. SQL
|
||||||
|
* Injection.
|
||||||
|
*/
|
||||||
|
public static boolean isSafePackageName(@Nullable String packageName) {
|
||||||
|
if (TextUtils.isEmpty(packageName)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
if (safePackageNamePattern == null) {
|
||||||
|
safePackageNamePattern = Pattern.compile("[a-zA-Z0-9._]+");
|
||||||
|
}
|
||||||
|
return safePackageNamePattern.matcher(packageName).matches();
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Calculate the number of days since the given date.
|
* Calculate the number of days since the given date.
|
||||||
*/
|
*/
|
||||||
|
@ -20,6 +20,10 @@
|
|||||||
package org.fdroid.fdroid.data;
|
package org.fdroid.fdroid.data;
|
||||||
|
|
||||||
import android.support.annotation.Nullable;
|
import android.support.annotation.Nullable;
|
||||||
|
import org.fdroid.fdroid.Utils;
|
||||||
|
|
||||||
|
import java.util.Arrays;
|
||||||
|
import java.util.List;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Represents action requests embedded in the index XML received from a repo.
|
* Represents action requests embedded in the index XML received from a repo.
|
||||||
@ -32,15 +36,33 @@ public class RepoPushRequest {
|
|||||||
|
|
||||||
public static final String INSTALL = "install";
|
public static final String INSTALL = "install";
|
||||||
public static final String UNINSTALL = "uninstall";
|
public static final String UNINSTALL = "uninstall";
|
||||||
|
public static final List<String> VALID_REQUESTS = Arrays.asList(INSTALL, UNINSTALL);
|
||||||
|
|
||||||
public final String request;
|
public final String request;
|
||||||
public final String packageName;
|
public final String packageName;
|
||||||
@Nullable
|
@Nullable
|
||||||
public final Integer versionCode;
|
public final Integer versionCode;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Create a new instance. {@code request} is validated against the list of
|
||||||
|
* valid install requests. {@code packageName} has a safety validation to
|
||||||
|
* make sure that only valid Android/Java Package Name characters are included.
|
||||||
|
* If validation fails, the the values are set to {@code null}, which are
|
||||||
|
* handled in {@link org.fdroid.fdroid.IndexV1Updater#processRepoPushRequests(List)}
|
||||||
|
* or {@link org.fdroid.fdroid.IndexUpdater#processRepoPushRequests(List)}
|
||||||
|
*/
|
||||||
public RepoPushRequest(String request, String packageName, @Nullable String versionCode) {
|
public RepoPushRequest(String request, String packageName, @Nullable String versionCode) {
|
||||||
|
if (VALID_REQUESTS.contains(request)) {
|
||||||
this.request = request;
|
this.request = request;
|
||||||
|
} else {
|
||||||
|
this.request = null;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (Utils.isSafePackageName(packageName)) {
|
||||||
this.packageName = packageName;
|
this.packageName = packageName;
|
||||||
|
} else {
|
||||||
|
this.packageName = null;
|
||||||
|
}
|
||||||
|
|
||||||
Integer i;
|
Integer i;
|
||||||
try {
|
try {
|
||||||
|
@ -160,6 +160,51 @@ public class RepoXMLHandlerTest {
|
|||||||
assertEquals(6, repoPushRequests.size());
|
assertEquals(6, repoPushRequests.size());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testPushRequestsRepoCorruption() {
|
||||||
|
RepoPushRequest repoPushRequest;
|
||||||
|
repoPushRequest = new RepoPushRequest(null, null, null); // request with no data
|
||||||
|
assertEquals(repoPushRequest.request, null);
|
||||||
|
assertEquals(repoPushRequest.packageName, null);
|
||||||
|
assertEquals(repoPushRequest.versionCode, null);
|
||||||
|
|
||||||
|
repoPushRequest = new RepoPushRequest("install", "org.fdroid.fdroid", "999999999999");
|
||||||
|
assertEquals(repoPushRequest.versionCode, null);
|
||||||
|
|
||||||
|
repoPushRequest = new RepoPushRequest("install", "org.fdroid.fdroid",
|
||||||
|
String.valueOf(((long) Integer.MAX_VALUE) + 1));
|
||||||
|
assertEquals(repoPushRequest.versionCode, null);
|
||||||
|
|
||||||
|
repoPushRequest = new RepoPushRequest("install", "org.fdroid.fdroid",
|
||||||
|
String.valueOf(((long) Integer.MIN_VALUE) - 1));
|
||||||
|
assertEquals(repoPushRequest.versionCode, null);
|
||||||
|
|
||||||
|
repoPushRequest = new RepoPushRequest("Robert'); DROP TABLE Students; --", "org.fdroid.fdroid", null);
|
||||||
|
assertEquals(repoPushRequest.request, null);
|
||||||
|
assertEquals(repoPushRequest.packageName, "org.fdroid.fdroid");
|
||||||
|
assertEquals(repoPushRequest.versionCode, null);
|
||||||
|
|
||||||
|
repoPushRequest = new RepoPushRequest("install", "Robert'); DROP TABLE Students; --", "123.1.1");
|
||||||
|
assertEquals(repoPushRequest.request, "install");
|
||||||
|
assertEquals(repoPushRequest.packageName, null);
|
||||||
|
assertEquals(repoPushRequest.versionCode, null);
|
||||||
|
|
||||||
|
repoPushRequest = new RepoPushRequest("install", "--", "123");
|
||||||
|
assertEquals(repoPushRequest.request, "install");
|
||||||
|
assertEquals(repoPushRequest.packageName, null);
|
||||||
|
assertEquals(repoPushRequest.versionCode, new Integer(123));
|
||||||
|
|
||||||
|
repoPushRequest = new RepoPushRequest("uninstall", "Robert'); DROP TABLE Students; --", "123");
|
||||||
|
assertEquals(repoPushRequest.request, "uninstall");
|
||||||
|
assertEquals(repoPushRequest.packageName, null);
|
||||||
|
assertEquals(repoPushRequest.versionCode, new Integer(123));
|
||||||
|
|
||||||
|
repoPushRequest = new RepoPushRequest("badrquest", "asdfasdfasdf", "123");
|
||||||
|
assertEquals(repoPushRequest.request, null);
|
||||||
|
assertEquals(repoPushRequest.packageName, "asdfasdfasdf");
|
||||||
|
assertEquals(repoPushRequest.versionCode, new Integer(123));
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testMediumRepo() {
|
public void testMediumRepo() {
|
||||||
Repo expectedRepo = new Repo();
|
Repo expectedRepo = new Repo();
|
||||||
|
Loading…
x
Reference in New Issue
Block a user