diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 02016b9a4..b6c2f09d1 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -27,6 +27,7 @@ stages: artifacts: name: "${CI_PROJECT_PATH}_${CI_JOB_STAGE}_${CI_COMMIT_REF_NAME}_${CI_COMMIT_SHA}" paths: + - kernel.log - logcat.txt - app/core* - app/*.log @@ -70,6 +71,10 @@ errorprone: - cat config/errorprone.gradle >> app/build.gradle - ./gradlew -Dorg.gradle.dependency.verification=lenient assembleDebug +# Run the tests in the emulator. Each step is broken out to run on +# its own since the CI runner can have limited RAM, and the emulator +# can take a while to start. +# # once these prove stable, the task should be switched to # connectedCheck to test all the build flavors .connected-template: &connected-template @@ -90,16 +95,14 @@ errorprone: - wait-for-emulator - adb devices - adb shell input keyevent 82 & + - ./gradlew installFullDebug + - adb shell am start -n org.fdroid.fdroid.debug/org.fdroid.fdroid.views.main.MainActivity - if [ $AVD_SDK -lt 25 ] || ! emulator -accel-check; then export FLAG=-Pandroid.testInstrumentationRunnerArguments.notAnnotation=androidx.test.filters.LargeTest; fi - ./gradlew connectedFullDebugAndroidTest $FLAG - || ./gradlew connectedFullDebugAndroidTest $FLAG - || ./gradlew connectedFullDebugAndroidTest $FLAG - || (adb -e logcat -d > logcat.txt; exit 1) -connected 22 default armeabi-v7a: - retry: 1 +no-accel 22 default x86: <<: *test-template <<: *connected-template @@ -107,15 +110,14 @@ connected 22 default armeabi-v7a: tags: - fdroid - kvm - allow_failure: true + only: + variables: + - $RUN_KVM_JOBS <<: *test-template <<: *connected-template -connected 26 google_apis x86: +kvm 29 microg x86_64: <<: *kvm-template - only: - - branches@fdroid/fdroidclient - - branches@eighthave/fdroidclient deploy_nightly: extends: .base diff --git a/app/build.gradle b/app/build.gradle index 3a933c96f..b222ba068 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -21,7 +21,7 @@ def basicApplicationId = "org.fdroid.basic" def privilegedExtensionApplicationId = '"org.fdroid.fdroid.privileged"' android { - compileSdkVersion 28 + compileSdkVersion 29 defaultConfig { versionCode 1012000 @@ -75,6 +75,8 @@ android { compileOptions { compileOptions.encoding = "UTF-8" + sourceCompatibility JavaVersion.VERSION_1_8 + targetCompatibility JavaVersion.VERSION_1_8 } aaptOptions { diff --git a/app/proguard-rules.pro b/app/proguard-rules.pro index f20d85241..045bbd72a 100644 --- a/app/proguard-rules.pro +++ b/app/proguard-rules.pro @@ -4,13 +4,11 @@ -keep class org.fdroid.fdroid.** {*;} -dontskipnonpubliclibraryclassmembers -dontwarn android.test.** --dontwarn com.android.support.test.** -dontwarn javax.naming.** -dontwarn org.slf4j.** -dontnote org.apache.http.** -dontnote android.net.http.** --dontnote android.support.** -dontnote **ILicensingService # Needed for espresso https://stackoverflow.com/a/21706087 @@ -48,4 +46,9 @@ public static final org.codehaus.jackson.annotate.JsonAutoDetect$Visibility *; } -keep public class your.class.** { *; +} + +# This is necessary so that RemoteWorkManager can be initialized (also marked with @Keep) +-keep class androidx.work.multiprocess.RemoteWorkManagerClient { + public (...); } \ No newline at end of file diff --git a/app/src/androidTest/java/org/fdroid/fdroid/MainActivityEspressoTest.java b/app/src/androidTest/java/org/fdroid/fdroid/MainActivityEspressoTest.java index ec0b1f0f0..080c64ef8 100644 --- a/app/src/androidTest/java/org/fdroid/fdroid/MainActivityEspressoTest.java +++ b/app/src/androidTest/java/org/fdroid/fdroid/MainActivityEspressoTest.java @@ -177,6 +177,7 @@ public class MainActivityEspressoTest { } @LargeTest + @Test public void showSettings() { ViewInteraction settingsBottonNavButton = onView( allOf(withText(R.string.menu_settings), isDisplayed())); @@ -211,6 +212,7 @@ public class MainActivityEspressoTest { } @LargeTest + @Test public void showUpdates() { ViewInteraction updatesBottonNavButton = onView(allOf(withText(R.string.main_menu__updates), isDisplayed())); updatesBottonNavButton.perform(click()); @@ -218,6 +220,7 @@ public class MainActivityEspressoTest { } @LargeTest + @Test public void startSwap() { if (!BuildConfig.FLAVOR.startsWith("full")) { return; @@ -233,6 +236,7 @@ public class MainActivityEspressoTest { } @LargeTest + @Test public void showCategories() { if (!BuildConfig.FLAVOR.startsWith("full")) { return; @@ -258,6 +262,7 @@ public class MainActivityEspressoTest { } @LargeTest + @Test public void showLatest() { if (!BuildConfig.FLAVOR.startsWith("full")) { return; @@ -280,6 +285,7 @@ public class MainActivityEspressoTest { } @LargeTest + @Test public void showSearch() { onView(allOf(withText(R.string.menu_settings), isDisplayed())).perform(click()); onView(withId(R.id.fab_search)).check(doesNotExist()); diff --git a/app/src/full/java/org/fdroid/fdroid/nearby/SwapService.java b/app/src/full/java/org/fdroid/fdroid/nearby/SwapService.java index e43b208ca..671edf710 100644 --- a/app/src/full/java/org/fdroid/fdroid/nearby/SwapService.java +++ b/app/src/full/java/org/fdroid/fdroid/nearby/SwapService.java @@ -17,14 +17,13 @@ import android.os.AsyncTask; import android.os.IBinder; import android.text.TextUtils; import android.util.Log; - import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.core.app.NotificationCompat; import androidx.core.app.ServiceCompat; import androidx.core.content.ContextCompat; import androidx.localbroadcastmanager.content.LocalBroadcastManager; - +import cc.mvdan.accesspoint.WifiApControl; import org.fdroid.fdroid.FDroidApp; import org.fdroid.fdroid.NotificationHelper; import org.fdroid.fdroid.Preferences; @@ -48,8 +47,6 @@ import java.util.Set; import java.util.Timer; import java.util.TimerTask; -import cc.mvdan.accesspoint.WifiApControl; - /** * Central service which manages all of the different moving parts of swap which are required * to enable p2p swapping of apps. @@ -429,7 +426,9 @@ public class SwapService extends Service { localBroadcastManager.unregisterReceiver(bonjourPeerFound); localBroadcastManager.unregisterReceiver(bonjourPeerRemoved); - unregisterReceiver(bluetoothScanModeChanged); + if (bluetoothAdapter != null) { + unregisterReceiver(bluetoothScanModeChanged); + } BluetoothManager.stop(this); diff --git a/app/src/full/res/layout/main_tab_swap.xml b/app/src/full/res/layout/main_tab_swap.xml index 1ca688aac..83f4926b5 100644 --- a/app/src/full/res/layout/main_tab_swap.xml +++ b/app/src/full/res/layout/main_tab_swap.xml @@ -18,7 +18,7 @@ app:layout_constraintBottom_toBottomOf="parent" app:layout_constraintTop_toBottomOf="@+id/both_parties_need_fdroid_text" android:layout_marginTop="36dp" - android:tint="?attr/mainTabSwapSplashTint" + app:tint="?attr/mainTabSwapSplashTint" android:scaleType="fitXY" tools:targetApi="jelly_bean"/> diff --git a/app/src/full/res/layout/swap_peer_list_item.xml b/app/src/full/res/layout/swap_peer_list_item.xml index 9f474e1cc..f98549b53 100644 --- a/app/src/full/res/layout/swap_peer_list_item.xml +++ b/app/src/full/res/layout/swap_peer_list_item.xml @@ -1,6 +1,7 @@ + app:tint="@color/swap_light_grey_icon"/> - - + * Must be called on App startup and after every proxy configuration change. */ public static void configureProxy(Preferences preferences) { @@ -676,4 +672,26 @@ public class FDroidApp extends Application { public static Context getInstance() { return instance; } + + /** + * Set up WorkManager on demand to avoid slowing down starts. + * + * @see CleanCacheWorker + * @see org.fdroid.fdroid.work.PopularityContestWorker + * @see org.fdroid.fdroid.work.UpdateWorker + * @see example + */ + @NonNull + @Override + public androidx.work.Configuration getWorkManagerConfiguration() { + if (BuildConfig.DEBUG) { + return new androidx.work.Configuration.Builder() + .setMinimumLoggingLevel(Log.DEBUG) + .build(); + } else { + return new androidx.work.Configuration.Builder() + .setMinimumLoggingLevel(Log.ERROR) + .build(); + } + } } diff --git a/app/src/main/java/org/fdroid/fdroid/Utils.java b/app/src/main/java/org/fdroid/fdroid/Utils.java index b9b101d48..34623bd1c 100644 --- a/app/src/main/java/org/fdroid/fdroid/Utils.java +++ b/app/src/main/java/org/fdroid/fdroid/Utils.java @@ -33,10 +33,6 @@ import android.os.Build; import android.os.Handler; import android.os.Looper; import android.os.StatFs; -import androidx.annotation.NonNull; -import androidx.annotation.Nullable; -import androidx.annotation.RequiresApi; -import androidx.swiperefreshlayout.widget.SwipeRefreshLayout; import android.text.Editable; import android.text.Html; import android.text.SpannableStringBuilder; @@ -52,6 +48,10 @@ import android.view.View; import android.view.ViewTreeObserver; import android.widget.ImageView; import android.widget.Toast; +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; +import androidx.annotation.RequiresApi; +import androidx.swiperefreshlayout.widget.SwipeRefreshLayout; import com.nostra13.universalimageloader.core.DisplayImageOptions; import com.nostra13.universalimageloader.core.ImageLoader; import com.nostra13.universalimageloader.core.assist.ImageScaleType; @@ -406,10 +406,26 @@ public final class Utils { return ret; } + /** * Get the fingerprint used to represent an APK signing key in F-Droid. * This is a custom fingerprint algorithm that was kind of accidentally * created, but is still in use. + * + * @see #getPackageSig(PackageInfo) + * @see org.fdroid.fdroid.data.Apk#sig + */ + public static String getsig(byte[] rawCertBytes) { + return Utils.hashBytes(toHexString(rawCertBytes).getBytes(), "md5"); + } + + /** + * Get the fingerprint used to represent an APK signing key in F-Droid. + * This is a custom fingerprint algorithm that was kind of accidentally + * created, but is still in use. + * + * @see #getsig(byte[]) + * @see org.fdroid.fdroid.data.Apk#sig */ public static String getPackageSig(PackageInfo info) { if (info == null || info.signatures == null || info.signatures.length < 1) { @@ -556,7 +572,7 @@ public final class Utils { } byte[] mdbytes = md.digest(); - return toHexString(mdbytes).toLowerCase(Locale.ENGLISH); + return toHexString(mdbytes); } catch (IOException e) { String message = e.getMessage(); if (message.contains("read failed: EIO (I/O error)")) { @@ -576,7 +592,7 @@ public final class Utils { * Computes the base 16 representation of the byte array argument. * * @param bytes an array of bytes. - * @return the bytes represented as a string of hexadecimal digits. + * @return the bytes represented as a string of lowercase hexadecimal digits. * @see source */ public static String toHexString(byte[] bytes) { @@ -589,7 +605,7 @@ public final class Utils { return new String(hexChars); } - private static final char[] HEX_LOOKUP_ARRAY = "0123456789ABCDEF".toCharArray(); + private static final char[] HEX_LOOKUP_ARRAY = "0123456789abcdef".toCharArray(); public static int parseInt(String str, int fallback) { if (str == null || str.length() == 0) { diff --git a/app/src/main/java/org/fdroid/fdroid/data/App.java b/app/src/main/java/org/fdroid/fdroid/data/App.java index 9e69bd899..3ba04b38f 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/App.java +++ b/app/src/main/java/org/fdroid/fdroid/data/App.java @@ -949,24 +949,7 @@ public class App extends ValueObject implements Comparable, Parcelable { } apkJar.close(); - /* - * I don't fully understand the loop used here. I've copied it verbatim - * from getsig.java bundled with FDroidServer. I *believe* it is taking - * the raw byte encoding of the certificate & converting it to a byte - * array of the hex representation of the original certificate byte - * array. This is then MD5 sum'd. It's a really bad way to be doing this - * if I'm right... If I'm not right, I really don't know! see lines - * 67->75 in getsig.java bundled with Fdroidserver - */ - final byte[] fdroidSig = new byte[rawCertBytes.length * 2]; - for (int j = 0; j < rawCertBytes.length; j++) { - byte v = rawCertBytes[j]; - int d = (v >> 4) & 0xF; - fdroidSig[j * 2] = (byte) (d >= 10 ? ('a' + d - 10) : ('0' + d)); - d = v & 0xF; - fdroidSig[j * 2 + 1] = (byte) (d >= 10 ? ('a' + d - 10) : ('0' + d)); - } - apk.sig = Utils.hashBytes(fdroidSig, "md5"); + apk.sig = Utils.getsig(rawCertBytes); } /** 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/main/res/layout-v11/app_permission_item.xml b/app/src/main/res/layout-v11/app_permission_item.xml index d7256a56e..7fd230b19 100644 --- a/app/src/main/res/layout-v11/app_permission_item.xml +++ b/app/src/main/res/layout-v11/app_permission_item.xml @@ -20,6 +20,7 @@ + app:tint="@android:color/black" /> diff --git a/app/src/test/java/org/fdroid/fdroid/UtilsTest.java b/app/src/test/java/org/fdroid/fdroid/UtilsTest.java index 1c6453d63..518d63dba 100644 --- a/app/src/test/java/org/fdroid/fdroid/UtilsTest.java +++ b/app/src/test/java/org/fdroid/fdroid/UtilsTest.java @@ -2,9 +2,9 @@ package org.fdroid.fdroid; import android.content.Context; - +import android.content.pm.PackageInfo; +import android.content.pm.Signature; import androidx.test.core.app.ApplicationProvider; - import org.fdroid.fdroid.views.AppDetailsRecyclerViewAdapter; import org.junit.Test; import org.junit.runner.RunWith; @@ -12,6 +12,7 @@ import org.robolectric.RobolectricTestRunner; import java.io.File; import java.util.Date; +import java.util.Random; import java.util.TimeZone; import static org.junit.Assert.assertEquals; @@ -215,4 +216,40 @@ public class UtilsTest { } } } + + /** + * Test the replacement for the ancient fingerprint algorithm. + * + * @see org.fdroid.fdroid.data.Apk#sig + */ + @Test + public void testGetsig() { + /* + * I don't fully understand the loop used here. I've copied it verbatim + * from getsig.java bundled with FDroidServer. I *believe* it is taking + * the raw byte encoding of the certificate & converting it to a byte + * array of the hex representation of the original certificate byte + * array. This is then MD5 sum'd. It's a really bad way to be doing this + * if I'm right... If I'm not right, I really don't know! see lines + * 67->75 in getsig.java bundled with Fdroidserver + */ + for (int length : new int[]{256, 345, 1233, 4032, 12092}) { + byte[] rawCertBytes = new byte[length]; + new Random().nextBytes(rawCertBytes); + final byte[] fdroidSig = new byte[rawCertBytes.length * 2]; + for (int j = 0; j < rawCertBytes.length; j++) { + byte v = rawCertBytes[j]; + int d = (v >> 4) & 0xF; + fdroidSig[j * 2] = (byte) (d >= 10 ? ('a' + d - 10) : ('0' + d)); + d = v & 0xF; + fdroidSig[j * 2 + 1] = (byte) (d >= 10 ? ('a' + d - 10) : ('0' + d)); + } + String sig = Utils.hashBytes(fdroidSig, "md5"); + assertEquals(sig, Utils.getsig(rawCertBytes)); + + PackageInfo packageInfo = new PackageInfo(); + packageInfo.signatures = new Signature[]{new Signature(rawCertBytes)}; + assertEquals(sig, Utils.getPackageSig(packageInfo)); + } + } } 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)); + } + } + +} diff --git a/config/checkstyle/checkstyle.xml b/config/checkstyle/checkstyle.xml index 69cf9e2c4..2631fe63e 100644 --- a/config/checkstyle/checkstyle.xml +++ b/config/checkstyle/checkstyle.xml @@ -115,7 +115,6 @@ -