Skip to content

Commit

Permalink
Merge branch 'hotfix/6.2.7'
Browse files Browse the repository at this point in the history
  • Loading branch information
federicoiosue committed May 30, 2023
2 parents 988df6d + 3ffe381 commit 4942ac4
Show file tree
Hide file tree
Showing 12 changed files with 181 additions and 41 deletions.
4 changes: 2 additions & 2 deletions gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions omniNotes/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/*
* Copyright (C) 2013-2023 Federico Iosue ([email protected])
*
* 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!"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -1067,7 +1066,7 @@ public Stats getStats() {
files++;
}
}
mStats.setAttachments(attachmentsAll);
mStats.setAttachments(attachments.size());
mStats.setImages(images);
mStats.setVideos(videos);
mStats.setAudioRecordings(audioRecordings);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* Copyright (C) 2013-2023 Federico Iosue ([email protected])
*
* 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()
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
}
}

}
}
2 changes: 1 addition & 1 deletion omniNotes/src/main/res/layout/activity_stats.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions omniNotes/src/main/res/raw/changelog.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down
1 change: 1 addition & 0 deletions omniNotes/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down

0 comments on commit 4942ac4

Please sign in to comment.