Guard against incorrect paths

[grzesiek2010]

Problem description

In PR #6340, we introduced a check to determine if paths were pointing to locations within the provided dirPath. This was further refined in PR #6357, and eventually, in PR #6363, the check was partially disabled, logging a non-fatal event instead of throwing an exception.

Thanks to PR #6363, we identified an issue with the original path comparison. Occasionally, after calling getCanonicalPath, the case of some letters in the path would change, such as:
/storage/emulated/0/android/data/org.odk.collect.android/files/projects/
instead of /storage/emulated/0/Android/data/org.odk.collect.android/files/projects/.

What we need to do

1. Revert the code from the androidshared module back to the shared module: 8f05e28
2. Reintroduce the SecurityException throw: f9377c4#diff-a8a1c2c9a690d0a3c02bd93865b8ee78dc4fea28e39dc1b59adf60437c71257eL14
3. Enhance path comparisons by making them case-insensitive: f9377c4#diff-a8a1c2c9a690d0a3c02bd93865b8ee78dc4fea28e39dc1b59adf60437c71257eR14

[grzesiek2010]

This is an issue with the paths returned by #getCanonicalPath. While the original paths start with storage/emulated/0/Android/data/ (the correct path), the paths returned by #getCanonicalPath may begin with variations like storage/emulated/0/android/data/ or storage/emulated/0/Android/Data/.

To address this while maintaining the case sensitivity of the Android OS, we can:

1. Check if the path returned by #getCanonicalPath starts with storage/emulated/0/Android/data/, ignoring case.
2. If step 1 returns true, update the canonical path to start with the correct format: storage/emulated/0/Android/data/ and use.
3. If step 1 returns false, throw an error, as this may indicate the potential vulnerability we were originally concerned with.

@lognaturel @seadowg if you have any thoughts please share.

[lognaturel]

When you say "and use" in step 2, you mean and use to do the rest of the path check, right? If so, that sounds right to me.

I think we should also write a stackoverflow post about this unexpected inconsistency in case so at least there's a record of it. I was going to do it but I think you should, @grzesiek2010!

I think it should point to the Android docs (https://developer.android.com/privacy-and-security/risks/path-traversal and https://developer.android.com/reference/java/io/File#getCanonicalPath()), say what we experienced when we did something like that (what you described above), and ask whether this is expected and if there's another recommended approach.

[grzesiek2010]

When you say "and use" in step 2, you mean and use to do the rest of the path check, right?

Yes, I was considering something like this:
c243c48

There are thousands of reports, and it's not feasible to review even 1% of them carefully. However, it seems that only the segment of the path containing /Android/data/ might vary, potentially appearing as /android/data/ or /android/Data/. Therefore, I plan to sanitize this segment.

For the beta release, there won’t be any major changes. I aim to avoid exceptions that could crash the app. My goal is simply to determine if sanitizing the /Android/data/ segment is sufficient.

[lognaturel]

That sounds great.

Are you up for writing the Stackoverflow post? The primary goal is to document this behavior in a way that's easy to find. A secondary goal would be to get an alternative workaround or assurance that what you're planning makes sense.

[grzesiek2010]

Yes, I can do that but my plan was to do that once we agree on my solution so that I can link the post in the comment above the fix and the other way around show on stackovrflow what's the problem and how we deal with it.