Closed Bug 817264 Opened 12 years ago Closed 11 years ago

systemXHR allows reading file: URLS

Categories

(Firefox OS Graveyard :: General, defect, P2)

All
Gonk (Firefox OS)
defect

Tracking

(blocking-basecamp:+, b2g18 fixed)

RESOLVED FIXED
B2G C3 (12dec-1jan)
blocking-basecamp +
Tracking Status
b2g18 --- fixed

People

(Reporter: amac, Assigned: dhylands)

References

Details

Attachments

(1 file)

Currently when using XHR with mozSystem: true, you can access the filesystem through the file: protocol. See for example:


https://github.com/AntonioMA/gaia/commit/daa0d954de01a619486699906e6c4e7d4794c538#L1R548

There I open the "/sdcard/certs" directory and it works although it supposedly shouldn't.
It appears that child processes have read access to the sdcard, that should not be the case.
Presumably that also means that they have write access.
blocking-basecamp: --- → ?
No.

I filed bug 777046 a while ago (and in reflection, it's probably fine).

The permissions on the sdcard are:

----rwxr-x system   sdcard_rw (for files)
d---rwxr-x system   sdcard_rw (for directories)

This says that users belonging to the sdcard_rw group are the only ones which have write access to the sdcard, and that everybody (except the system user) has read access.
blocking-basecamp: ? → +
Flags: needinfo?(overholt)
Great that there's no write access!

We should not be accessing the sdcard from the child process at all right now though, other than through bugs like the one in comment 0.

So it should be safe to limit read access to only the sdcard_rw group.
Summary: systemXHR allows accessing file: URLS → systemXHR allows reading file: URLS
Michael, is this appropriate to do? Triage drivers think this can only be handled with a kernel rebuild.
Assignee: nobody → mwu
Can we get a priority and target milestone for this>
Priority: -- → P1
Target Milestone: --- → B2G C3 (12dec-1jan)
Michael, can you provide any info here?  Saying you're not the right owner is also fine :)
Flags: needinfo?(overholt) → needinfo?(mwu)
Sorry about the delay - I haven't had a chance to look at the issue till now.

The permission in question is here:
http://androidxref.com/4.0.4/xref/system/vold/Volume.cpp#424

So a change in vold would be necessary to change the permission. Vold is not in the kernel so no kernel repack is necessary.
Flags: needinfo?(mwu)
I agree that eliminating read access to sdcard_rw is a good thing. Ideally, we'd just chroot the whole thing and not even worry about this, but it looks like bug 776648 won't make v1. I don't think this counts as a P1 blocker, though.
Priority: P1 → P2
Thanks for the info, Michael.  Re-nomming for discussing today.
blocking-basecamp: + → ?
We discussed this again during triage and decided it's still a blocker.  Dave volunteered to take on the work.
Assignee: mwu → dhylands
blocking-basecamp: ? → +
I can see 3 ways of resolving this:

1 - change system/vold/Volume.cpp (only a single character change)
2 - Hack the mount command in bionic
3 - Do some LD_PRELOAD magic when loading vold to intercept the mount command

What are people's preference for this?
Flags: needinfo?(mvines)
We should be able to put in a patch for this. Just put up a patch and let mvines know.
Yep, #1 sgtm.  We can certainly patch this in our tree.
Flags: needinfo?(mvines)
This patch removes read & execute priviledge from "others".

Note that this patch should definitely NOT be taken until such time as the blocking bug is resolved (Right now, deviceStorage can't see any files on the sdcard once this bug is applied)
Depends on: 823191
And the patch is intended to be applied inside the B2G/system/vold directory (which is a git repository)
I hope that bug 824172 would resolve this additionally, but maybe not.

Anyway, dep is fixed and merged.  m1, good to go.  (ni? to highlight in your email queue.)
Flags: needinfo?(mvines)
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Sg, will pick this up soon
Flags: needinfo?(mvines)
(landing this now on CAF)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: