[Webkit-unassigned] [Bug 199367] [GTK][WPE] Explicitly blacklist problematic directories for sandbox

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=199367

Michael Catanzaro <mcatanzaro at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #373250|commit-queue?               |commit-queue-
              Flags|                            |

--- Comment #11 from Michael Catanzaro <mcatanzaro at igalia.com> ---
Comment on attachment 373250
  --> https://bugs.webkit.org/attachment.cgi?id=373250
Patch

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

OK, only nits remaining from me.

I'll wait until tomorrow before giving r+ in case Carlos Garcia wants any changes.

> Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp:1190
> +    /* These are backend specific though the blacklist covers all for consistent support */

// The directories that won't work are specific to the sandbox backends, but the blacklist covers all of them for consistency.

Note: //

> Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp:1195
> +        "tmp", /* This doesn't work with flatpak-spawn */
> +        /* The rest of these are re-created by the bwrap sandbox in both cases and don't make sense */
> +        "sys", "proc", "dev",
> +        /* A value of just `/` also doesn't make sense */

Besides changing the comments from /* to //, also be sure to end them with a period to form a complete sentence. This is a WebKit style requirement.

> Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp:1199
> +    /* NOTE: Due to previous check there is always one leading `/` */

Ditto.

> Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp:1214
> + * Paths in directories such as `/tmp`, `/sys`, `/proc`, and `/dev` or all of `/` are not valid.

Wrap at 80 chars

> Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp:1227
> +        g_critical("Path added to sandbox is blacklisted (%s)", path);

How about: "Attempted to add disallowed path to sandbox: %s"

Might as well also first do:

if (!g_path_is_absolute(path)) {
    g_critical("Attempted to add relative path to sandbox, but paths must be absolute");
    return;
}

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20190701/5ed0de67/attachment.html>