diff --git a/app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java b/app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java index 5aef95ac7..3da55ec34 100644 --- a/app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java +++ b/app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java @@ -344,8 +344,7 @@ public class RepoXMLHandler extends DefaultHandler { repoDescription = cleanWhiteSpace(attributes.getValue("", "description")); repoTimestamp = parseLong(attributes.getValue("", "timestamp"), 0); repoIcon = attributes.getValue("", "icon"); - } else if (RepoPushRequest.INSTALL.equals(localName) - || RepoPushRequest.UNINSTALL.equals(localName)) { + } else if (RepoPushRequest.VALID_REQUESTS.contains(localName)) { if (repo.pushRequests == Repo.PUSH_REQUEST_ACCEPT_ALWAYS) { RepoPushRequest r = new RepoPushRequest( localName, diff --git a/app/src/main/java/org/fdroid/fdroid/Utils.java b/app/src/main/java/org/fdroid/fdroid/Utils.java index e99b9cab4..110838af2 100644 --- a/app/src/main/java/org/fdroid/fdroid/Utils.java +++ b/app/src/main/java/org/fdroid/fdroid/Utils.java @@ -76,6 +76,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; import java.util.concurrent.TimeUnit; +import java.util.regex.Pattern; public final class Utils { @@ -98,6 +99,8 @@ public final class Utils { private static DisplayImageOptions.Builder defaultDisplayImageOptionsBuilder; private static DisplayImageOptions repoAppDisplayImageOptions; + private static Pattern safePackageNamePattern; + public static final String FALLBACK_ICONS_DIR = "/icons/"; /* @@ -621,6 +624,21 @@ public final class Utils { 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. */ diff --git a/app/src/main/java/org/fdroid/fdroid/data/RepoPushRequest.java b/app/src/main/java/org/fdroid/fdroid/data/RepoPushRequest.java index 49082c873..7ac81ae95 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/RepoPushRequest.java +++ b/app/src/main/java/org/fdroid/fdroid/data/RepoPushRequest.java @@ -20,6 +20,10 @@ package org.fdroid.fdroid.data; 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. @@ -32,15 +36,33 @@ public class RepoPushRequest { public static final String INSTALL = "install"; public static final String UNINSTALL = "uninstall"; + public static final List VALID_REQUESTS = Arrays.asList(INSTALL, UNINSTALL); public final String request; public final String packageName; @Nullable 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) { - this.request = request; - this.packageName = packageName; + if (VALID_REQUESTS.contains(request)) { + this.request = request; + } else { + this.request = null; + } + + if (Utils.isSafePackageName(packageName)) { + this.packageName = packageName; + } else { + this.packageName = null; + } Integer i; try { diff --git a/app/src/test/java/org/fdroid/fdroid/updater/RepoXMLHandlerTest.java b/app/src/test/java/org/fdroid/fdroid/updater/RepoXMLHandlerTest.java index 7b568c579..a0168659f 100644 --- a/app/src/test/java/org/fdroid/fdroid/updater/RepoXMLHandlerTest.java +++ b/app/src/test/java/org/fdroid/fdroid/updater/RepoXMLHandlerTest.java @@ -160,6 +160,51 @@ public class RepoXMLHandlerTest { 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 public void testMediumRepo() { Repo expectedRepo = new Repo();