sanitize all packageNames from the index

This is insurance to make sure that packageNames are not abused for
exploiting F-Droid.  The database queries already use SQL Prepared
Statements, but who know what else might be exploitable.

fdroid/fdroidclient#1588

app/src/main/java/org/fdroid/fdroid/data/Apk.java

@@ -460,6 +460,20 @@ public class Apk extends ValueObject implements Comparable<Apk>, Parcelable {
         return null;
     }
 
+    /**
+     * Set the Package Name property while ensuring it is sanitized.
+     */
+    @JsonProperty("packageName")
+    @SuppressWarnings("unused")
+    void setPackageName(String packageName) {
+        if (Utils.isSafePackageName(packageName)) {
+            this.packageName = packageName;
+        } else {
+            throw new IllegalArgumentException("Repo index package entry includes unsafe packageName: '"
+                    + packageName + "'");
+        }
+    }
+
     @JsonProperty("uses-permission")
     @SuppressWarnings("unused")
     private void setUsesPermission(Object[][] permissions) {

app/src/main/java/org/fdroid/fdroid/data/App.java

@@ -406,6 +406,19 @@ public class App extends ValueObject implements Comparable<App>, Parcelable {
         this.description = formatDescription(description);
     }
 
+    /**
+     * Set the Package Name property while ensuring it is sanitized.
+     */
+    @JsonProperty("packageName")
+    void setPackageName(String packageName) {
+        if (Utils.isSafePackageName(packageName)) {
+            this.packageName = packageName;
+        } else {
+            throw new IllegalArgumentException("Repo index app entry includes unsafe packageName: '"
+                    + packageName + "'");
+        }
+    }
+
     /**
      * Parses the {@code localized} block in the incoming index metadata,
      * choosing the best match in terms of locale/language while filling as

app/src/main/java/org/fdroid/fdroid/data/RepoXMLHandler.java

@@ -22,7 +22,6 @@ package org.fdroid.fdroid.data;
 import android.os.Build;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
-import org.fdroid.fdroid.IndexUpdater;
 import org.fdroid.fdroid.Utils;
 import org.fdroid.fdroid.data.Schema.ApkTable;
 import org.xml.sax.Attributes;
@@ -330,8 +329,8 @@ public class RepoXMLHandler extends DefaultHandler {
     }
 
     @Override
-    public void startElement(String uri, String localName, String qName,
-                             Attributes attributes) throws SAXException {
+    public void startElement(String uri, String localName, String qName, Attributes attributes)
+            throws SAXException {
         super.startElement(uri, localName, qName, attributes);
 
         if ("repo".equals(localName)) {
@@ -353,7 +352,11 @@ public class RepoXMLHandler extends DefaultHandler {
         } else if ("application".equals(localName) && curapp == null) {
             curapp = new App();
             curapp.repoId = repo.getId();
-            curapp.packageName = attributes.getValue("", "id");
+            try {
+                curapp.setPackageName(attributes.getValue("", "id"));
+            } catch (IllegalArgumentException e) {
+                throw new SAXException(e);
+            }
 
             // To appease the NON NULL constraint in the DB. Usually there is a description, and it
             // is quite difficult to get an app to _not_ have a description when using fdroidserver.

app/src/test/java/org/fdroid/fdroid/data/RepoXMLHandlerTest.java

@@ -27,12 +27,18 @@ import android.text.TextUtils;
 import android.util.Log;
 import org.apache.commons.io.FileUtils;
 import org.fdroid.fdroid.BuildConfig;
+import org.fdroid.fdroid.mock.MockRepo;
 import org.fdroid.fdroid.mock.RepoDetails;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.robolectric.RobolectricTestRunner;
 import org.robolectric.annotation.Config;
+import org.xml.sax.InputSource;
+import org.xml.sax.XMLReader;
 
+import javax.xml.parsers.SAXParser;
+import javax.xml.parsers.SAXParserFactory;
+import java.io.BufferedInputStream;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
@@ -48,6 +54,7 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 @Config(constants = BuildConfig.class)
 @RunWith(RobolectricTestRunner.class)
@@ -124,6 +131,33 @@ public class RepoXMLHandlerTest {
         });
     }
 
+    @Test(expected = IllegalArgumentException.class)
+    public void testSimpleIndexWithCorruptedPackageName() throws Throwable {
+        Repo expectedRepo = new Repo();
+        expectedRepo.name = "F-Droid";
+        expectedRepo.signingCertificate = "308201ee30820157a0030201020204300d845b300d06092a864886f70d01010b0500302a3110300e060355040b1307462d44726f6964311630140603550403130d70616c6174736368696e6b656e301e170d3134303432373030303633315a170d3431303931323030303633315a302a3110300e060355040b1307462d44726f6964311630140603550403130d70616c6174736368696e6b656e30819f300d06092a864886f70d010101050003818d0030818902818100a439472e4b6d01141bfc94ecfe131c7c728fdda670bb14c57ca60bd1c38a8b8bc0879d22a0a2d0bc0d6fdd4cb98d1d607c2caefbe250a0bd0322aedeb365caf9b236992fac13e6675d3184a6c7c6f07f73410209e399a9da8d5d7512bbd870508eebacff8b57c3852457419434d34701ccbf692267cbc3f42f1c5d1e23762d790203010001a321301f301d0603551d0e041604140b1840691dab909746fde4bfe28207d1cae15786300d06092a864886f70d01010b05000381810062424c928ffd1b6fd419b44daafef01ca982e09341f7077fb865905087aeac882534b3bd679b51fdfb98892cef38b63131c567ed26c9d5d9163afc775ac98ad88c405d211d6187bde0b0d236381cc574ba06ef9080721a92ae5a103a7301b2c397eecc141cc850dd3e123813ebc41c59d31ddbcb6e984168280c53272f6a442b"; // NOCHECKSTYLE LineLength
+        expectedRepo.description = "The official repository of the F-Droid client. Applications in this repository are either official binaries built by the original application developers, or are binaries built from source by the admin of f-droid.org using the tools on https://gitorious.org/f-droid.";  // NOCHECKSTYLE LineLength
+        expectedRepo.timestamp = 1398733213;
+
+        InputStream inputStream = getClass().getClassLoader()
+                .getResourceAsStream("simpleIndexWithCorruptedPackageName.xml");
+        SAXParserFactory factory = SAXParserFactory.newInstance();
+        factory.setNamespaceAware(true);
+        SAXParser parser = factory.newSAXParser();
+        XMLReader reader = parser.getXMLReader();
+        RepoDetails repoDetails = new RepoDetails();
+        MockRepo mockRepo = new MockRepo(100, Repo.PUSH_REQUEST_IGNORE);
+        RepoXMLHandler handler = new RepoXMLHandler(mockRepo, repoDetails);
+        reader.setContentHandler(handler);
+        InputSource is = new InputSource(new BufferedInputStream(inputStream));
+        try {
+            reader.parse(is);
+        } catch (org.xml.sax.SAXException e) {
+            throw e.getCause();
+        }
+        fail();
+    }
+
     @Test
     public void testPushRequestsRepoIgnore() {
         Repo expectedRepo = new Repo();
@@ -188,17 +222,17 @@ public class RepoXMLHandlerTest {
         repoPushRequest = new RepoPushRequest("install", "--", "123");
         assertEquals(repoPushRequest.request, "install");
         assertEquals(repoPushRequest.packageName, null);
-        assertEquals(repoPushRequest.versionCode, new Integer(123));
+        assertEquals(repoPushRequest.versionCode, Integer.valueOf(123));
 
         repoPushRequest = new RepoPushRequest("uninstall", "Robert'); DROP TABLE Students; --", "123");
         assertEquals(repoPushRequest.request, "uninstall");
         assertEquals(repoPushRequest.packageName, null);
-        assertEquals(repoPushRequest.versionCode, new Integer(123));
+        assertEquals(repoPushRequest.versionCode, Integer.valueOf(123));
 
         repoPushRequest = new RepoPushRequest("badrquest", "asdfasdfasdf", "123");
         assertEquals(repoPushRequest.request, null);
         assertEquals(repoPushRequest.packageName, "asdfasdfasdf");
-        assertEquals(repoPushRequest.versionCode, new Integer(123));
+        assertEquals(repoPushRequest.versionCode, Integer.valueOf(123));
     }
 
     @Test

app/src/test/java/org/fdroid/fdroid/updater/IndexV1UpdaterTest.java

@@ -8,6 +8,7 @@ import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.JsonMappingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.ObjectReader;
 import org.apache.commons.io.IOUtils;
@@ -159,6 +160,26 @@ public class IndexV1UpdaterTest extends FDroidProviderTest {
         getClass().getResourceAsStream("foo");
     }
 
+    @Test(expected = IllegalArgumentException.class)
+    public void testIndexV1WithCorruptAppPackageName() throws Throwable {
+        try {
+            testBadTestyJar("testy.at.or.at_corrupt_app_package_name_index-v1.jar");
+        } catch (JsonMappingException e) {
+            throw e.getCause();
+        }
+        fail();
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testIndexV1WithCorruptPackageName() throws Throwable {
+        try {
+            testBadTestyJar("testy.at.or.at_corrupt_package_name_index-v1.jar");
+        } catch (JsonMappingException e) {
+            throw e.getCause();
+        }
+        fail();
+    }
+
     @Test(expected = IndexUpdater.SigningException.class)
     public void testIndexV1WithBadTestyJarNoManifest() throws IOException, IndexUpdater.UpdateException {
         testBadTestyJar("testy.at.or.at_no-MANIFEST.MF_index-v1.jar");