Merge branch 'hotfix/6.2.7'

Author: federicoiosue

gradle.properties

@@ -14,8 +14,8 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
-VERSION_NAME=6.2.6
-VERSION_CODE=321
+VERSION_NAME=6.2.7
+VERSION_CODE=322
 PACKAGE=it.feio.android.omninotes
 MIN_SDK=21
 TARGET_SDK=31

omniNotes/build.gradle

@@ -173,8 +173,6 @@ dependencies {
         transitive = true
         exclude group: "com.android.support"
     }
-    implementation 'com.jakewharton:butterknife:10.2.1'
-    annotationProcessor 'com.jakewharton:butterknife-compiler:10.2.1'
     implementation('org.mnode.ical4j:ical4j:3.0.11') {
         exclude group: 'commons.io'
     }

omniNotes/src/androidTest/java/it/feio/android/omninotes/testutils/BaseAndroidTestCase.java

@@ -188,9 +188,8 @@ protected static void assertUtilityClassWellDefined(final Class<?> clazz)
   }
 
   protected static void assertUtilityClassWellDefined(final Class<?> clazz,
-      boolean weakClassModifier,
-      boolean weakConstructorModifier)
-      throws NoSuchMethodException, InvocationTargetException, InstantiationException, IllegalAccessException {
+      boolean weakClassModifier, boolean weakConstructorModifier)
+      throws NoSuchMethodException, InstantiationException, IllegalAccessException {
     if (!weakClassModifier) {
       assertTrue("class must be final", Modifier.isFinal(clazz.getModifiers()));
     }

omniNotes/src/androidTest/java/it/feio/android/omninotes/utils/SecurityTest.kt

@@ -0,0 +1,88 @@
+/*
+ * Copyright (C) 2013-2023 Federico Iosue (federico@iosue.it)
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package it.feio.android.omninotes.utils
+
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import it.feio.android.omninotes.exceptions.checked.ContentSecurityException
+import it.feio.android.omninotes.testutils.BaseAndroidTestCase
+import it.feio.android.omninotes.utils.Security.Companion.decrypt
+import it.feio.android.omninotes.utils.Security.Companion.encrypt
+import it.feio.android.omninotes.utils.Security.Companion.validatePath
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotEquals
+import org.junit.Assert.assertThrows
+import org.junit.Test
+import org.junit.runner.RunWith
+
+@RunWith(AndroidJUnit4::class)
+class SecurityTest : BaseAndroidTestCase() {
+    private val LOREM = ("Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor"
+            + " incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco"
+            + " laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit "
+            + "esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa "
+            + "qui officia deserunt mollit anim ID est laborum.")
+
+    @Test
+    @Throws(Exception::class)
+    fun checkUtilityClassWellDefined() {
+        assertUtilityClassWellDefined(Security::class.java)
+    }
+
+    @Test
+    fun encrypt() {
+        assertNotEquals(TEXT, encrypt(TEXT, PASS))
+    }
+
+    @Test
+    fun decrypt() {
+        val encryptedText = encrypt(TEXT, PASS)
+
+        assertEquals(TEXT, decrypt(encryptedText, PASS))
+        assertNotEquals(TEXT, decrypt(encryptedText, "zaza$PASS"))
+    }
+
+    @Test
+    fun decryptUnencrypted() {
+        assertNotEquals(0, decrypt(LOREM, PASS)!!.length.toLong())
+    }
+
+    @Test
+    fun validatePath_startsWithData() {
+        val path = "file:///data/data/it.feio.android.omninotes.foss/shared_prefs/it.feio.android.omninotes.foss_preferences.xml"
+
+        assertThrows(ContentSecurityException::class.java) { validatePath(path) }
+    }
+
+    @Test
+    fun validatePath_pathTraversal() {
+        val path = "file:///storage/emulated/0/../../../data/data/it.feio.android.omninotes.foss/shared_prefs/it.feio.android.omninotes.foss_preferences.xml"
+
+        assertThrows(ContentSecurityException::class.java) { validatePath(path) }
+    }
+
+    @Test
+    fun validatePath_valid() {
+        val path = "/images/screenshot/16844742322307525633366385236595.jpg"
+
+        validatePath(path)
+    }
+
+    companion object {
+        private const val PASS = "12345uselessPasswords"
+        private const val TEXT = "Today is a - good - day to test useless things!"
+    }
+}

omniNotes/src/main/java/it/feio/android/omninotes/DetailFragment.java

@@ -61,6 +61,7 @@
 import static it.feio.android.omninotes.utils.ConstantsBase.SWIPE_MARGIN;
 import static it.feio.android.omninotes.utils.ConstantsBase.SWIPE_OFFSET;
 import static it.feio.android.omninotes.utils.ConstantsBase.THUMBNAIL_SIZE;
+import static it.feio.android.omninotes.utils.Security.validatePath;
 import static java.lang.Integer.parseInt;
 import static java.lang.Long.parseLong;
 
@@ -133,6 +134,7 @@
 import it.feio.android.omninotes.async.notes.SaveNoteTask;
 import it.feio.android.omninotes.databinding.FragmentDetailBinding;
 import it.feio.android.omninotes.db.DbHelper;
+import it.feio.android.omninotes.exceptions.checked.ContentSecurityException;
 import it.feio.android.omninotes.exceptions.checked.UnhandledIntentException;
 import it.feio.android.omninotes.helpers.AttachmentsHelper;
 import it.feio.android.omninotes.helpers.IntentHelper;
@@ -167,6 +169,7 @@
 import it.feio.android.omninotes.utils.KeyboardUtils;
 import it.feio.android.omninotes.utils.PasswordHelper;
 import it.feio.android.omninotes.utils.ReminderHelper;
+import it.feio.android.omninotes.utils.Security;
 import it.feio.android.omninotes.utils.ShortcutHelper;
 import it.feio.android.omninotes.utils.StorageHelper;
 import it.feio.android.omninotes.utils.TagsHelper;
@@ -551,27 +554,38 @@ private void handleIntents() {
   }
 
   private void importAttachments(Intent i) {
-
     if (!i.hasExtra(Intent.EXTRA_STREAM)) {
       return;
     }
 
     if (i.getExtras().get(Intent.EXTRA_STREAM) instanceof Uri) {
       Uri uri = i.getParcelableExtra(Intent.EXTRA_STREAM);
       // Google Now passes Intent as text but with audio recording attached the case must be handled like this
-      if (!INTENT_GOOGLE_NOW.equals(i.getAction())) {
+      if (validatePath(uri.getPath()) && !INTENT_GOOGLE_NOW.equals(i.getAction())) {
         String name = FileHelper.getNameFromUri(mainActivity, uri);
         new AttachmentTask(this, uri, name, this).execute();
       }
     } else {
       ArrayList<Uri> uris = i.getParcelableArrayListExtra(Intent.EXTRA_STREAM);
       for (Uri uriSingle : uris) {
-        String name = FileHelper.getNameFromUri(mainActivity, uriSingle);
-        new AttachmentTask(this, uriSingle, name, this).execute();
+        if (validatePath(uriSingle.getPath())) {
+          String name = FileHelper.getNameFromUri(mainActivity, uriSingle);
+          new AttachmentTask(this, uriSingle, name, this).execute();
+        }
       }
     }
   }
 
+  private boolean validatePath(String path) {
+    try {
+      Security.validatePath(path);
+      return true;
+    } catch (ContentSecurityException e) {
+      mainActivity.showMessage(R.string.insecure_content_found, ONStyle.WARN);
+      return false;
+    }
+  }
+
   @SuppressLint("NewApi")
   private void initViews() {
     // Sets onTouchListener to the whole activity to swipe notes

omniNotes/src/main/java/it/feio/android/omninotes/async/MainMenuTask.java

@@ -24,17 +24,15 @@
 import android.content.res.TypedArray;
 import android.os.AsyncTask;
 import androidx.fragment.app.Fragment;
-import butterknife.BindView;
-import butterknife.ButterKnife;
 import com.pixplicity.easyprefs.library.Prefs;
 import de.greenrobot.event.EventBus;
 import it.feio.android.omninotes.MainActivity;
 import it.feio.android.omninotes.R;
 import it.feio.android.omninotes.async.bus.NavigationUpdatedEvent;
+import it.feio.android.omninotes.databinding.FragmentNavigationDrawerBinding;
 import it.feio.android.omninotes.models.NavigationItem;
 import it.feio.android.omninotes.models.adapters.NavDrawerAdapter;
 import it.feio.android.omninotes.models.misc.DynamicNavigationLookupTable;
-import it.feio.android.omninotes.models.views.NonScrollableListView;
 import it.feio.android.omninotes.utils.Navigation;
 import java.lang.ref.WeakReference;
 import java.util.ArrayList;
@@ -43,18 +41,15 @@
 
 public class MainMenuTask extends AsyncTask<Void, Void, List<NavigationItem>> {
 
-  private final WeakReference<Fragment> mFragmentWeakReference;
+  private final WeakReference<Fragment> fragmentWeakReference;
   private final MainActivity mainActivity;
-  @BindView(R.id.drawer_nav_list)
-  NonScrollableListView mDrawerList;
-  @BindView(R.id.drawer_tag_list)
-  NonScrollableListView mDrawerCategoriesList;
 
+  FragmentNavigationDrawerBinding navDrawer;
 
-  public MainMenuTask(Fragment mFragment) {
-    mFragmentWeakReference = new WeakReference<>(mFragment);
-    this.mainActivity = (MainActivity) mFragment.getActivity();
-    ButterKnife.bind(this, mFragment.getView());
+  public MainMenuTask(Fragment fragment) {
+    fragmentWeakReference = new WeakReference<>(fragment);
+    mainActivity = (MainActivity) fragment.getActivity();
+    navDrawer = FragmentNavigationDrawerBinding.inflate(fragment.getLayoutInflater());
   }
 
   @Override
@@ -65,33 +60,31 @@ protected List<NavigationItem> doInBackground(Void... params) {
   @Override
   protected void onPostExecute(final List<NavigationItem> items) {
     if (isAlive()) {
-      mDrawerList.setAdapter(new NavDrawerAdapter(mainActivity, items));
-      mDrawerList.setOnItemClickListener((arg0, arg1, position, arg3) -> {
-        String navigation = mFragmentWeakReference.get().getResources().getStringArray(R.array
+      navDrawer.drawerNavList.setAdapter(new NavDrawerAdapter(mainActivity, items));
+      navDrawer.drawerNavList.setOnItemClickListener((arg0, arg1, position, arg3) -> {
+        String navigation = fragmentWeakReference.get().getResources().getStringArray(R.array
             .navigation_list_codes)[items.get(position).getArrayIndex()];
         updateNavigation(position, navigation);
       });
-      mDrawerList.justifyListViewHeightBasedOnChildren();
+      navDrawer.drawerNavList.justifyListViewHeightBasedOnChildren();
     }
   }
 
   private void updateNavigation(int position, String navigation) {
     if (mainActivity.updateNavigation(navigation)) {
-      mDrawerList.setItemChecked(position, true);
-      if (mDrawerCategoriesList != null) {
-        mDrawerCategoriesList.setItemChecked(0, false); // Called to force redraw
-      }
+      navDrawer.drawerNavList.setItemChecked(position, true);
+      navDrawer.drawerTagList.setItemChecked(0, false); // Called to force redraw
       mainActivity.getIntent().setAction(Intent.ACTION_MAIN);
       EventBus.getDefault()
-          .post(new NavigationUpdatedEvent(mDrawerList.getItemAtPosition(position)));
+          .post(new NavigationUpdatedEvent(navDrawer.drawerNavList.getItemAtPosition(position)));
     }
   }
 
   private boolean isAlive() {
-    return mFragmentWeakReference.get() != null
-        && mFragmentWeakReference.get().isAdded()
-        && mFragmentWeakReference.get().getActivity() != null
-        && !mFragmentWeakReference.get().getActivity().isFinishing();
+    return fragmentWeakReference.get() != null
+        && fragmentWeakReference.get().isAdded()
+        && fragmentWeakReference.get().getActivity() != null
+        && !fragmentWeakReference.get().getActivity().isFinishing();
   }
 
   private List<NavigationItem> buildMainMenu() {

omniNotes/src/main/java/it/feio/android/omninotes/db/DbHelper.java

@@ -1046,7 +1046,6 @@ public Stats getStats() {
     mStats.setCharsAvg(avgChars);
 
     // Everything about attachments
-    int attachmentsAll = 0;
     int images = 0;
     int videos = 0;
     int audioRecordings = 0;
@@ -1067,7 +1066,7 @@ public Stats getStats() {
         files++;
       }
     }
-    mStats.setAttachments(attachmentsAll);
+    mStats.setAttachments(attachments.size());
     mStats.setImages(images);
     mStats.setVideos(videos);
     mStats.setAudioRecordings(audioRecordings);

omniNotes/src/main/java/it/feio/android/omninotes/exceptions/checked/ContentSecurityException.kt

@@ -0,0 +1,20 @@
+/*
+ * Copyright (C) 2013-2023 Federico Iosue (federico@iosue.it)
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package it.feio.android.omninotes.exceptions.checked
+
+class ContentSecurityException(message: String) : SecurityException()

omniNotes/src/main/java/it/feio/android/omninotes/utils/Security.kt

@@ -17,15 +17,20 @@
 
 package it.feio.android.omninotes.utils
 
+import android.net.Uri
 import android.util.Base64
+import it.feio.android.omninotes.exceptions.checked.ContentSecurityException
 import it.feio.android.omninotes.helpers.LogDelegate
 import java.nio.charset.StandardCharsets
 import java.security.MessageDigest
 import java.security.NoSuchAlgorithmException
-import javax.crypto.*
+import javax.crypto.Cipher
+import javax.crypto.SecretKeyFactory
 import javax.crypto.spec.DESKeySpec
+import kotlin.jvm.Throws
 
-class Security private constructor(){
+
+class Security private constructor() {
 
     companion object {
         @JvmStatic
@@ -82,5 +87,14 @@ class Security private constructor(){
             }
         }
 
+        @JvmStatic
+        @Throws(ContentSecurityException::class)
+        fun validatePath(path: String?) {
+            val uri = Uri.parse(path).path
+            if (uri?.startsWith("/data")!! || uri.contains("../")) {
+                throw ContentSecurityException("Invalid")
+            }
+        }
+
     }
 }

omniNotes/src/main/res/layout/activity_stats.xml

@@ -271,7 +271,7 @@
         android:gravity="end"
         android:paddingEnd="7dp"
         android:paddingStart="7dp"
-        android:text="@string/stats_attachments"
+        android:text="@string/stats_images"
         android:textStyle="bold"></TextView>
 
       <TextView

omniNotes/src/main/res/raw/changelog.xml

@@ -16,6 +16,20 @@
   -->
 <changelog bulletedList="true">
 
+  <changelogversion
+    changeDate="May 31, 2023"
+    versionName="6.2.7">
+    <changelogtext>[u]Fix[/u] Security path traversal vulnerability</changelogtext>
+    <changelogtext>[u]Fix[/u] Attachments' count in statistics (thanks to S.Vago)</changelogtext>
+  </changelogversion>
+
+  <changelogversion
+    changeDate="Apr 30, 2023"
+    versionName="6.2.6">
+    <changelogtext>[i]Improved![/i] Reduced required permissions and removed some legacy code</changelogtext>
+    <changelogtext>[u]Fix[/u] Minor backup related fixes</changelogtext>
+  </changelogversion>
+
   <changelogversion
     changeDate="Apr 20, 2023"
     versionName="6.2.5">

omniNotes/src/main/res/values/strings.xml

@@ -231,6 +231,7 @@
   <string name="undo_changes_note_confirmation">Confirm undoing changes?</string>
   <string name="done">Done</string>
   <string name="nothing_selected">Nothing selected</string>
+  <string name="insecure_content_found">Some of the received content was dangerous, it has been ignored</string>
 
   <!-- Settings -->
   <string name="dashclock_description">Displays some statistics and expiring reminders from Omni Notes</string>