Code review comment for lp://qastaging/~kyrofa/snap-confine/create_user_data_directory

Revision history for this message
Tyler Hicks (tyhicks) wrote :

Thanks Kyle - This looks pretty good now but there's still one issue. Jamie tells me that snaps were granted control of the paths specified in SNAP_USER_DATA and SNAP_APP_USER_DATA_PATH, as well as SNAP_USER_DATA/../ and SNAP_APP_USER_DATA_PATH/../, because of the bug that you're trying to fix here. This change would happily follow symlinks that snaps had created in those locations since there's nothing to prevent the following of symlinks.

I'd suggest modifying the mkdir() loop to use openat() w/ O_NOFOLLOW and mkdirat(), as Seth originally suggested.

We'll need to update ubuntu-core-security, at the same time or after this change lands in the image, to remove access to those directories.

Yes, those white space changes sure make it more difficult to review! :)

review: Needs Fixing

« Back to merge proposal