[webkit-reviews] review denied: [Bug 90005] [WK2] Putting QtWebProcess into a chrooted sandbox : [Attachment 174415] Proposed patch

[bugzilla-daemon at webkit.org]

Andreas Kling <kling at webkit.org> has denied  review:
Bug 90005: [WK2] Putting QtWebProcess into a chrooted sandbox
https://bugs.webkit.org/show_bug.cgi?id=90005

Attachment 174415: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=174415&action=review

------- Additional Comments from Andreas Kling <kling at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=174415&action=review


I'm still of the opinion that rolling your own string functions for code whose
sole purpose is increasing security is insane.

This version is *still* storing the size_t result of str(n)len in int
variables, leaving the code open to arithmetic overflow bugs. And this is just
one problem, I'm sure there are more of them hiding in the code.

Also, why do you need to send messages back and forth to communicate with the
clone() child? Can't you just waitpid() for it and check the exit code? Seems
like that would decrease the attack surface of this helper considerably.

> Source/WebKit2/Configurations/FeatureDefines.xcconfig:136
> +ENABLE_SUID_SANDBOX_LINUX = ;

Why add this to the XCode build? It's not ENABLE_SUID_SANDBOX_LINUX will ever
be used on Mac.