Bug 186110 – Add a sandbox profile for com.cisco.webex.plugin.gpc64 plugin

• Status: RESOLVED
• Resolution: FIXED
• Component: Plug-ins

[youenn fablet]

Add a sandbox profile for com.cisco.webex.plugin.gpc64 plugin

[youenn fablet]

Created attachment 341625 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/40728353>

[youenn fablet]

Comment on attachment 341625 [details]
Patch

I think that we might be able to strengthen the sandbox.
It might best be done when having some real testing.

[Brent Fulgham]

Comment on attachment 341625 [details]
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:44
> +

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:46
> +(allow process-fork)

Lots of powerful operations allowed :-(

> Source/WebKit/Resources/PlugInSandboxProfiles/com.cisco.webex.plugin.gpc64.sb:74
> +(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:79
> +(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:80
> +(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:81
> +(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:82
> +(allow sysctl-write)

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

[youenn fablet]

I will try to do further testing to ensure I can remove some of these potentially non useful rules.

[youenn fablet]

Created attachment 341789 [details]
Patch

[youenn fablet]

OK, this sandbox should allow plugin listing but there are still some issues with running the plugin.
In particular, it seems that the finalizer in com.apple.WebKit.plugin-common.sb.in is not compatible.

[youenn fablet]

Created attachment 341798 [details]
Patch

[youenn fablet]

Created attachment 341799 [details]
Patch

[youenn fablet]

I removed some potentially unneeded rules.
I kept the port rules unchanged for now since webex might use a wide range.

[Brent Fulgham]

Comment on attachment 341799 [details]
Patch

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

Looks much better! I think this is a good first cut. We can tighten it as we work with it more.

> Source/WebKit/PluginProcess/mac/com.apple.WebKit.plugin-common.sb.in:531
> +            (if (not (defined? 'allow-symlinks))

I just tested this locally and made sure existing plugins still hit this code path, so that works properly.

> Source/WebKit/Resources/PlugInSandboxProfiles/com.cisco.webex.plugin.gpc64.sb:41
> +    (prefix "/private/tmp"))

It's a little scary to be granting global read/write to "/private/tmp". It would be nice to tighten this up to a sub-folder if possible.

[youenn fablet]

> > Source/WebKit/Resources/PlugInSandboxProfiles/com.cisco.webex.plugin.gpc64.sb:41
> > +    (prefix "/private/tmp"))
> 
> It's a little scary to be granting global read/write to "/private/tmp". It
> would be nice to tighten this up to a sub-folder if possible.

An initial version of the sandbox was using /private/tmp subfolders but I saw some sandbox issues.
I think we will need to continue fixing the sandbox, I'll add a FIXME in the meantime.

[youenn fablet]

Created attachment 341809 [details]
Patch for landing

[EWS Watchlist]

Comment on attachment 341809 [details]
Patch for landing

Attachment 341809 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/7939894

New failing tests:
http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-audio.html

[EWS Watchlist]

Created attachment 341828 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[WebKit Commit Bot]

Comment on attachment 341809 [details]
Patch for landing

Clearing flags on attachment: 341809

Committed r232436: <https://trac.webkit.org/changeset/232436>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.