From 615e559ce1fda174f26880cfd528a53e50f9d8d1 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Fri, 13 Apr 2018 00:20:52 +0200 Subject: [PATCH] only prompt to swap back to proper swap URLs Before, it was possible to annoy the user by sending HTTP POST with any repo URL in it. --- .../fdroid/net/BluetoothDownloader.java | 10 ++++++ .../org/fdroid/fdroid/net/HttpDownloader.java | 15 +++++--- .../views/swap/SwapWorkflowActivity.java | 19 +++++++--- app/src/main/res/values/strings.xml | 1 + .../fdroid/fdroid/net/HttpDownloaderTest.java | 36 +++++++++++++++++++ 5 files changed, 73 insertions(+), 8 deletions(-) create mode 100644 app/src/test/java/org/fdroid/fdroid/net/HttpDownloaderTest.java diff --git a/app/src/main/java/org/fdroid/fdroid/net/BluetoothDownloader.java b/app/src/main/java/org/fdroid/fdroid/net/BluetoothDownloader.java index 85d4f4d84..cb4f54abf 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/BluetoothDownloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/BluetoothDownloader.java @@ -14,7 +14,12 @@ import org.fdroid.fdroid.net.bluetooth.httpish.Response; import java.io.File; import java.io.IOException; import java.io.InputStream; +import java.util.regex.Pattern; +/** + * Download from a Bluetooth swap repo. Example URI: + * {@code bluetooth://84-CF-BF-8B-3E-34/fdroid/repo} + */ public class BluetoothDownloader extends Downloader { private static final String TAG = "BluetoothDownloader"; @@ -23,6 +28,11 @@ public class BluetoothDownloader extends Downloader { private FileDetails fileDetails; private final String sourcePath; + public static boolean isBluetoothUri(Uri uri) { + return SCHEME.equals(uri.getScheme()) + && Pattern.matches("([0-9A-F]{2}-)+[0-9A-F]{2}", uri.getHost()); + } + public BluetoothDownloader(Uri uri, File destFile) throws IOException { super(uri, destFile); String macAddress = uri.getHost().replace("-", ":"); diff --git a/app/src/main/java/org/fdroid/fdroid/net/HttpDownloader.java b/app/src/main/java/org/fdroid/fdroid/net/HttpDownloader.java index 880d48fa5..619f48c00 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/HttpDownloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/HttpDownloader.java @@ -122,16 +122,23 @@ public class HttpDownloader extends Downloader { cacheTag = connection.getHeaderField(HEADER_FIELD_ETAG); } - private boolean isSwapUrl() { - String host = sourceUrl.getHost(); - return sourceUrl.getPort() > 1023 // only root can use <= 1023, so never a swap repo + public static boolean isSwapUrl(Uri uri) { + return isSwapUrl(uri.getHost(), uri.getPort()); + } + + public static boolean isSwapUrl(URL url) { + return isSwapUrl(url.getHost(), url.getPort()); + } + + public static boolean isSwapUrl(String host, int port) { + return port > 1023 // only root can use <= 1023, so never a swap repo && host.matches("[0-9.]+") // host must be an IP address && FDroidApp.subnetInfo.isInRange(host); // on the same subnet as we are } private HttpURLConnection getConnection() throws SocketTimeoutException, IOException { HttpURLConnection connection; - if (isSwapUrl()) { + if (isSwapUrl(sourceUrl)) { // swap never works with a proxy, its unrouted IP on the same subnet connection = (HttpURLConnection) sourceUrl.openConnection(); } else { diff --git a/app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java b/app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java index 4bbfbd4f1..f889ca834 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java +++ b/app/src/main/java/org/fdroid/fdroid/views/swap/SwapWorkflowActivity.java @@ -29,10 +29,9 @@ import android.view.MenuInflater; import android.view.View; import android.view.ViewGroup; import android.widget.Toast; - +import cc.mvdan.accesspoint.WifiApControl; import com.google.zxing.integration.android.IntentIntegrator; import com.google.zxing.integration.android.IntentResult; - import org.fdroid.fdroid.BuildConfig; import org.fdroid.fdroid.FDroidApp; import org.fdroid.fdroid.NfcHelper; @@ -47,6 +46,8 @@ import org.fdroid.fdroid.installer.Installer; import org.fdroid.fdroid.localrepo.LocalRepoManager; import org.fdroid.fdroid.localrepo.SwapService; import org.fdroid.fdroid.localrepo.peers.Peer; +import org.fdroid.fdroid.net.BluetoothDownloader; +import org.fdroid.fdroid.net.HttpDownloader; import java.util.Arrays; import java.util.Date; @@ -57,8 +58,6 @@ import java.util.Set; import java.util.Timer; import java.util.TimerTask; -import cc.mvdan.accesspoint.WifiApControl; - /** * This activity will do its best to show the most relevant screen about swapping to the user. * The problem comes when there are two competing goals - 1) Show the user a list of apps from another @@ -209,8 +208,20 @@ public class SwapWorkflowActivity extends AppCompatActivity { showRelevantView(); } + /** + * Check whether incoming {@link Intent} is a swap repo, and ensure that + * it is a valid swap URL. The hostname can only be either an IP or + * Bluetooth address. + */ private void checkIncomingIntent() { Intent intent = getIntent(); + Uri uri = intent.getData(); + if (uri != null && !HttpDownloader.isSwapUrl(uri) && !BluetoothDownloader.isBluetoothUri(uri)) { + String msg = getString(R.string.swap_toast_invalid_url, uri); + Toast.makeText(this, msg, Toast.LENGTH_LONG).show(); + return; + } + if (intent.getBooleanExtra(EXTRA_CONFIRM, false) && !intent.getBooleanExtra(EXTRA_SWAP_INTENT_HANDLED, false)) { // Storing config in this variable will ensure that when showRelevantView() is next // run, it will show the connect swap view (if the service is available). diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index a38fd786a..24456d40f 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -460,6 +460,7 @@ This often occurs with apps installed via Google Play or other sources, if they Error occurred while connecting to device, can\'t swap with it! Swapping not enabled Before swapping, your device must be made visible. + Invalid URL for swapping: %1$s needs access to Do you want to install an update diff --git a/app/src/test/java/org/fdroid/fdroid/net/HttpDownloaderTest.java b/app/src/test/java/org/fdroid/fdroid/net/HttpDownloaderTest.java new file mode 100644 index 000000000..d5aee05c8 --- /dev/null +++ b/app/src/test/java/org/fdroid/fdroid/net/HttpDownloaderTest.java @@ -0,0 +1,36 @@ +package org.fdroid.fdroid.net; + +import android.net.Uri; +import org.apache.commons.net.util.SubnetUtils; +import org.fdroid.fdroid.BuildConfig; +import org.fdroid.fdroid.FDroidApp; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; + +import java.net.MalformedURLException; +import java.net.URL; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertFalse; + +@Config(constants = BuildConfig.class, sdk = 24) +@RunWith(RobolectricTestRunner.class) +@SuppressWarnings("LineLength") +public class HttpDownloaderTest { + + @Test + public void testIsSwapUri() throws MalformedURLException { + FDroidApp.subnetInfo = new SubnetUtils("192.168.0.112/24").getInfo(); + String urlString = "http://192.168.0.112:8888/fdroid/repo?fingerprint=113F56CBFA967BA825DD13685A06E35730E0061C6BB046DF88A"; + assertTrue(HttpDownloader.isSwapUrl("192.168.0.112", 8888)); // NOPMD + assertTrue(HttpDownloader.isSwapUrl(Uri.parse(urlString))); + assertTrue(HttpDownloader.isSwapUrl(new URL(urlString))); + + assertFalse(HttpDownloader.isSwapUrl("192.168.1.112", 8888)); // NOPMD + assertFalse(HttpDownloader.isSwapUrl("192.168.0.112", 80)); // NOPMD + assertFalse(HttpDownloader.isSwapUrl(Uri.parse("https://malware.com:8888"))); + assertFalse(HttpDownloader.isSwapUrl(new URL("https://www.google.com"))); + } +}