From 018e3221a7529bfe57e07f7a40026c7e540a0654 Mon Sep 17 00:00:00 2001
From: Hans-Christoph Steiner <hans@eds.org>
Date: Wed, 10 Feb 2021 23:05:55 +0100
Subject: [PATCH] prevent search terms triggering SQL injection vulns

---
 .../fdroid/views/main/MainActivity.java       | 19 +++++++++++---
 .../fdroid/fdroid/data/SanitizedFileTest.java | 10 ++++++++
 .../fdroid/views/main/MainActivityTest.java   | 25 +++++++++++++++++++
 3 files changed, 50 insertions(+), 4 deletions(-)
 create mode 100644 app/src/test/java/org/fdroid/fdroid/views/main/MainActivityTest.java

diff --git a/app/src/main/java/org/fdroid/fdroid/views/main/MainActivity.java b/app/src/main/java/org/fdroid/fdroid/views/main/MainActivity.java
index d01af3979..b026b39cd 100644
--- a/app/src/main/java/org/fdroid/fdroid/views/main/MainActivity.java
+++ b/app/src/main/java/org/fdroid/fdroid/views/main/MainActivity.java
@@ -37,7 +37,6 @@ import android.view.ViewGroup;
 import android.widget.LinearLayout;
 import android.widget.TextView;
 import android.widget.Toast;
-
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.appcompat.app.AppCompatActivity;
@@ -45,11 +44,9 @@ import androidx.core.content.ContextCompat;
 import androidx.localbroadcastmanager.content.LocalBroadcastManager;
 import androidx.recyclerview.widget.LinearLayoutManager;
 import androidx.recyclerview.widget.RecyclerView;
-
 import com.ashokvarma.bottomnavigation.BottomNavigationBar;
 import com.ashokvarma.bottomnavigation.BottomNavigationItem;
 import com.ashokvarma.bottomnavigation.TextBadgeItem;
-
 import org.fdroid.fdroid.AppUpdateStatusManager;
 import org.fdroid.fdroid.AppUpdateStatusManager.AppUpdateStatus;
 import org.fdroid.fdroid.BuildConfig;
@@ -305,6 +302,11 @@ public class MainActivity extends AppCompatActivity implements BottomNavigationB
 
     }
 
+    /**
+     * Since any app could send this {@link Intent}, and the search terms are
+     * fed into a SQL query, the data must be strictly sanitized to avoid
+     * SQL injection attacks.
+     */
     private void handleSearchOrAppViewIntent(Intent intent) {
         if (Intent.ACTION_SEARCH.equals(intent.getAction())) {
             String query = intent.getStringExtra(SearchManager.QUERY);
@@ -391,6 +393,8 @@ public class MainActivity extends AppCompatActivity implements BottomNavigationB
         }
 
         if (!TextUtils.isEmpty(packageName)) {
+            // sanitize packageName to be a valid Java packageName and prevent exploits
+            packageName = packageName.replaceAll("[^A-Za-z\\d_.]", "");
             Utils.debugLog(TAG, "FDroid launched via app link for '" + packageName + "'");
             Intent intentToInvoke = new Intent(this, AppDetailsActivity.class);
             intentToInvoke.putExtra(AppDetailsActivity.EXTRA_APPID, packageName);
@@ -402,12 +406,19 @@ public class MainActivity extends AppCompatActivity implements BottomNavigationB
         }
     }
 
+    /**
+     * These strings might end up in a SQL query, so strip all non-alpha-num
+     */
+    static String sanitizeSearchTerms(String query) {
+        return query.replaceAll("[^\\p{L}\\d_ -]", " ");
+    }
+
     /**
      * Initiates the {@link AppListActivity} with the relevant search terms passed in via the query arg.
      */
     private void performSearch(String query) {
         Intent searchIntent = new Intent(this, AppListActivity.class);
-        searchIntent.putExtra(AppListActivity.EXTRA_SEARCH_TERMS, query);
+        searchIntent.putExtra(AppListActivity.EXTRA_SEARCH_TERMS, sanitizeSearchTerms(query));
         startActivity(searchIntent);
     }
 
diff --git a/app/src/test/java/org/fdroid/fdroid/data/SanitizedFileTest.java b/app/src/test/java/org/fdroid/fdroid/data/SanitizedFileTest.java
index fc4dca32b..beed13102 100644
--- a/app/src/test/java/org/fdroid/fdroid/data/SanitizedFileTest.java
+++ b/app/src/test/java/org/fdroid/fdroid/data/SanitizedFileTest.java
@@ -5,6 +5,7 @@ import org.junit.Test;
 import java.io.File;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assume.assumeTrue;
 
 public class SanitizedFileTest {
@@ -45,4 +46,13 @@ public class SanitizedFileTest {
 
     }
 
+    @Test
+    public void testSanitizeFileName() {
+        for (String valid : new String[]{"An.stop", "a.0", "packageName", "com.this-and-that", "A_.o"}) {
+            assertEquals(valid, SanitizedFile.sanitizeFileName(valid));
+        }
+        for (String invalid : new String[]{"'--;DROP", "a.0)", "packageName\n"}) {
+            assertNotEquals(invalid, SanitizedFile.sanitizeFileName(invalid));
+        }
+    }
 }
diff --git a/app/src/test/java/org/fdroid/fdroid/views/main/MainActivityTest.java b/app/src/test/java/org/fdroid/fdroid/views/main/MainActivityTest.java
new file mode 100644
index 000000000..4725a2464
--- /dev/null
+++ b/app/src/test/java/org/fdroid/fdroid/views/main/MainActivityTest.java
@@ -0,0 +1,25 @@
+package org.fdroid.fdroid.views.main;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+
+public class MainActivityTest {
+
+    @Test
+    public void testSanitizeSearchTerms() {
+        for (String valid : new String[]{"private browser", "πÇÇ", "现代 通用字", "български", "عربي"}) {
+            assertEquals(valid, MainActivity.sanitizeSearchTerms(valid));
+        }
+        for (String invalid : new String[]{
+                "Robert'); DROP TABLE Students; --",
+                "xxx') OR 1 = 1 -- ]",
+                "105 OR 1=1;",
+                "\" OR \"=\"",
+        }) {
+            assertNotEquals(invalid, MainActivity.sanitizeSearchTerms(invalid));
+        }
+    }
+
+}