[webkit-reviews] review denied: [Bug 186110] Add a sandbox profile for com.cisco.webex.plugin.gpc64 plugin : [Attachment 341625] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has denied youenn fablet
<youennf at gmail.com>'s request for review:
Bug 186110: Add a sandbox profile for com.cisco.webex.plugin.gpc64 plugin
https://bugs.webkit.org/show_bug.cgi?id=186110

Attachment 341625: Patch

https://bugs.webkit.org/attachment.cgi?id=341625&action=review




--- Comment #4 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 341625
  --> https://bugs.webkit.org/attachment.cgi?id=341625
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=341625&action=review

I think this is very close, but I'd like to see some tighter rules on the rules
I mentioned above. Maybe you've found that WebEx just won't work without these
open fully -- if so, let me know.

>
Source/WebKit/Resources/PlugInSandboxProfiles/com.cisco.webex.plugin.gpc64.sb:4
4
> +

You might consider preventing symlinks from being created, unless WebEx uses
them:

(if (defined? 'vnode-type)
	(deny file-write-create (vnode-type SYMLINK)))

This would help protect against a compromised plugin from creating a symlink
someplace bad.

>
Source/WebKit/Resources/PlugInSandboxProfiles/com.cisco.webex.plugin.gpc64.sb:4
6
> +(allow process-fork)

Lots of powerful operations allowed :-(

>
Source/WebKit/Resources/PlugInSandboxProfiles/com.cisco.webex.plugin.gpc64.sb:7
4
> +(allow network-outbound)

I wonder if these outbound connections could be limited to specific ports?

E.g, like we do in some other plugins:

(allow network-outbound
   (remote udp "*:4160" "*:88"))

>
Source/WebKit/Resources/PlugInSandboxProfiles/com.cisco.webex.plugin.gpc64.sb:7
9
> +(allow ipc-posix-shm)

Do you need all ipc-posix-shm? For example, could you just have
"ipc-posix-shm-read-data"? Even better would be to limit it to specific IPC
agents you want to talk to.

>
Source/WebKit/Resources/PlugInSandboxProfiles/com.cisco.webex.plugin.gpc64.sb:8
0
> +(allow mach-lookup)

It would be much better to limit mach-lookup to just the mach endpoints you
actually need. This is a huge window for attackers.

>
Source/WebKit/Resources/PlugInSandboxProfiles/com.cisco.webex.plugin.gpc64.sb:8
1
> +(allow sysctl-read)

Can this be limited to a smaller subset?

(allow sysctl-read
    (sysctl-name
	"hw.byteorder"
	"hw.busfrequency_max"
	...
 (sysctl-name-regex #"^net.routetable")
)

>
Source/WebKit/Resources/PlugInSandboxProfiles/com.cisco.webex.plugin.gpc64.sb:8
2
> +(allow sysctl-write)

Oh gosh -- can this be limited to specific things? All of WebContent process
manages to avoid sysctl-write for anything!