prevent search terms triggering SQL injection vulns

This commit is contained in:
Hans-Christoph Steiner 2021-02-10 23:05:55 +01:00
parent 38e4b05e56
commit 018e3221a7
3 changed files with 50 additions and 4 deletions

View File

@ -37,7 +37,6 @@ import android.view.ViewGroup;
import android.widget.LinearLayout; import android.widget.LinearLayout;
import android.widget.TextView; import android.widget.TextView;
import android.widget.Toast; import android.widget.Toast;
import androidx.annotation.NonNull; import androidx.annotation.NonNull;
import androidx.annotation.Nullable; import androidx.annotation.Nullable;
import androidx.appcompat.app.AppCompatActivity; import androidx.appcompat.app.AppCompatActivity;
@ -45,11 +44,9 @@ import androidx.core.content.ContextCompat;
import androidx.localbroadcastmanager.content.LocalBroadcastManager; import androidx.localbroadcastmanager.content.LocalBroadcastManager;
import androidx.recyclerview.widget.LinearLayoutManager; import androidx.recyclerview.widget.LinearLayoutManager;
import androidx.recyclerview.widget.RecyclerView; import androidx.recyclerview.widget.RecyclerView;
import com.ashokvarma.bottomnavigation.BottomNavigationBar; import com.ashokvarma.bottomnavigation.BottomNavigationBar;
import com.ashokvarma.bottomnavigation.BottomNavigationItem; import com.ashokvarma.bottomnavigation.BottomNavigationItem;
import com.ashokvarma.bottomnavigation.TextBadgeItem; import com.ashokvarma.bottomnavigation.TextBadgeItem;
import org.fdroid.fdroid.AppUpdateStatusManager; import org.fdroid.fdroid.AppUpdateStatusManager;
import org.fdroid.fdroid.AppUpdateStatusManager.AppUpdateStatus; import org.fdroid.fdroid.AppUpdateStatusManager.AppUpdateStatus;
import org.fdroid.fdroid.BuildConfig; 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) { private void handleSearchOrAppViewIntent(Intent intent) {
if (Intent.ACTION_SEARCH.equals(intent.getAction())) { if (Intent.ACTION_SEARCH.equals(intent.getAction())) {
String query = intent.getStringExtra(SearchManager.QUERY); String query = intent.getStringExtra(SearchManager.QUERY);
@ -391,6 +393,8 @@ public class MainActivity extends AppCompatActivity implements BottomNavigationB
} }
if (!TextUtils.isEmpty(packageName)) { 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 + "'"); Utils.debugLog(TAG, "FDroid launched via app link for '" + packageName + "'");
Intent intentToInvoke = new Intent(this, AppDetailsActivity.class); Intent intentToInvoke = new Intent(this, AppDetailsActivity.class);
intentToInvoke.putExtra(AppDetailsActivity.EXTRA_APPID, packageName); 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. * Initiates the {@link AppListActivity} with the relevant search terms passed in via the query arg.
*/ */
private void performSearch(String query) { private void performSearch(String query) {
Intent searchIntent = new Intent(this, AppListActivity.class); Intent searchIntent = new Intent(this, AppListActivity.class);
searchIntent.putExtra(AppListActivity.EXTRA_SEARCH_TERMS, query); searchIntent.putExtra(AppListActivity.EXTRA_SEARCH_TERMS, sanitizeSearchTerms(query));
startActivity(searchIntent); startActivity(searchIntent);
} }

View File

@ -5,6 +5,7 @@ import org.junit.Test;
import java.io.File; import java.io.File;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assume.assumeTrue; import static org.junit.Assume.assumeTrue;
public class SanitizedFileTest { 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));
}
}
} }

View File

@ -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));
}
}
}