We need to allow symlinks in our whitelist to allow whitelisting locations like /etc/localtime that could be a symlink to anywhere. Currently symlinks don't work because they're followed by the code that checks if access is permitted, so also follow them when adding the permission. Security consequence: an attacker that has already owned your computer can give the web process additional permissions by creating a symlink from a permissible loaction to an impermissible location. (Not a problem.)
Created attachment 249279 [details] [Linux] Seccomp Filters: Canonicalize filesystem path when whitelisting it
Created attachment 249281 [details] [Linux] Improvements to SyscallPolicy
Comment on attachment 249281 [details] [Linux] Improvements to SyscallPolicy Dammit, wrong bug again... this time it's my fault for typing the bug number wrong, though.
Created attachment 249285 [details] [Linux] Seccomp Filters: Canonicalize filesystem path when whitelisting it
Comment on attachment 249285 [details] [Linux] Seccomp Filters: Canonicalize filesystem path when whitelisting it View in context: https://bugs.webkit.org/attachment.cgi?id=249285&action=review > Source/WebKit2/Shared/linux/SeccompFilters/SyscallPolicy.h:59 > + static void addPermissionToMap(const String& path, Permission, PermissionMap&); Why use a private static method? Why not just implement this as a normal helper method?
I only see one good reason: to keep SyscallPolicy::PermissionMap private.
But the helper method can be private, just as the static one is now.
Comment on attachment 249285 [details] [Linux] Seccomp Filters: Canonicalize filesystem path when whitelisting it View in context: https://bugs.webkit.org/attachment.cgi?id=249285&action=review > Source/WebKit2/Shared/linux/SeccompFilters/SyscallPolicy.cpp:125 > + SyscallPolicy::addPermissionToMap(path, permission, m_filePermission); This could still directly set the HashMap entry, only calling out a helper method (or some already existing API?) that canonicalizes the given path.
(In reply to comment #7) > But the helper method can be private, just as the static one is now. Oh, the reason to implement it as a static member function instead of a non-static member function is that it doesn't require access to any member variables. I can make it non-static if you prefer that style, but that's not my preference. (In reply to comment #8) > This could still directly set the HashMap entry, only calling out a helper > method (or some already existing API?) that canonicalizes the given path. OK, I agree that would be nicer.
Created attachment 257286 [details] Patch
Committed r187190: <http://trac.webkit.org/changeset/187190>