[Webkit-unassigned] [Bug 193439] [GTK][WPE] Add enable-javascript-markup setting

Mon Jan 21 00:00:35 PST 2019

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

--- Comment #9 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to Michael Catanzaro from comment #8)
> Comment on attachment 359147 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=359147&action=review
> 
> Nice test.
> 
> > Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:1458
> > +     * Determines whether or not JavaScript markup is allowed in document. When this setting is disabled,
> 
> documents.

I mean the page document.

> > Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:1459
> > +     * all JavaScript related elements and attributes are removed from the document during parsing. Note that
> 
> JavaScript-related
> 
> > Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:1460
> > +     * all JavaScript related elements and attributes are removed from the document during parsing. Note that
> > +     * executing JavaScript is still allowed if #WebKitSettings:enable-javascript is %TRUE.
> 
> I agree with Adrian that this documentation in insufficient. Basically all
> of his questions should be clarified by better documentation.

Ok, I'm open to suggestions.

> But I have another concern. Executing JavaScript is still allowed... how?

I think this is pretty clear too... JavaScript execution is not disallowed at all, the script elements and attributes are re moved from the document, that's the only thing.

> "Use scripts, js api in the web extension, user message handlers, etc." OK,
> but those are API requests, and I've separately suggests that API requests
> should always ignore #WebKitSettings:enable-javascript and execute anyway.

Yes, that's also desirable.

> This looked like something we could change on a cross-platform basis. So
> then maybe this new setting is not needed after all, if it, in practice,
> just blocks all JS except API requests?

That's a good question, I'm adding Geoffrey to the CC, since he suggested to use this setting instead of enable-js.

> I really don't like mentioning that
> executing JavaScript is still allowed via API if
> #WebKitSettings:enable-javascript is %TRUE here, because I want to change
> that! Basically my fear is that "enable-javascript-markup" is the setting
> that most applications will want to use, and "enable-javascript" will be a
> wrong trap choice. So I'd rather make that usable first. Would this setting
> still be useful after that change?

If removing js markup from document is a better approach we could just deprecate the other setting. I find more confusing a setting that disables js, but still allows it for API stuff, than a setting that removes js markup from document (but it seems this is only clear enough for me).

> r- to improve the docs and continue this discussion. The code is fine, of
> course.
> 
> >>>>> Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:3582
> >>>>> + * Returns: %TRUE If JavaScript markup is enabled or %FALSE otherwise.
> >>>> 
> >>>> What is “JavaScript Markup”? A specification, a WebKit specific concept?
> >>>> Something widely understood among Web developers?... I think the documentation
> >>>> for this setting needs an explanation of what the option does, as it is nearly
> >>>> impossible to find anything about this topic that would seem relevant when
> >>>> implementing an application which makes use of WebKit.
> >>>> 
> >>>> After some digging in the repository history I arrived at bug #112999 and
> >>>> bug #113122 and it took me wading through a pile of comments and reading
> >>>> some bits of the code to guess what the setting does. So please let's make
> >>>> the API documentation better by adding something in the lines of:
> >>>> 
> >>>>   “Enabling this setting will strip <script> tags from loaded HTML
> >>>>    content, but other forms of JavaScript execution e.g. using
> >>>>    webkit_web_view_run_javascript() are still allowed. This setting
> >>>>    is intended for applications which display HTML content but are
> >>>>    not full browsers, and which want to avoid the risk of script
> >>>>    injection attacks, as is the case of applications like e-mail
> >>>>    and news readers.”
> >>>> 
> >>>> If possible it should be more concrete that the above explaining what
> >>>> gets restricted and what not, because very often we programmers end up
> >>>> introducing accidental security vulnerabilities due to assumptions we
> >>>> make caused by incomplete documentation of third party code used.
> >>> 
> >>> The explanation in the patch is even more accurate and complete than this one, I would say:
> >>> 
> >>> +     * Determines whether or not JavaScript markup is allowed in document. When this setting is disabled,
> >>> +     * all JavaScript related elements and attributes are removed from the document during parsing. Note that
> >>> +     * executing JavaScript is still allowed if #WebKitSettings:enable-javascript is %TRUE.
> >>> 
> >>> It's not only about script tags but also even listener attributes like onload and other js related attributes. That's clear in the current explanation. It also says that elements are removed while parsing and that js execution is still possible (not only run_js, but any js execution).
> >> 
> >> TBH, with this writing it is not completely clear to me what the
> >> setting does. What does “all JavaScript related elements and attributes”
> >> mean? For example it does not answer question like:
> >> 
> >>  - Can the JS DOM API be used to insert a new <script> tag?
> >>  - If a <script> tag can be inserted using the DOM, will it work for
> >>    <script src="..."> or only for elements with inline JS code in them? 
> >>  - Will <script> tags inside nested frames or an <iframe> be loaded
> >>    and executed?
> >> 
> >> (and that's only from the top of my head, I could come up with more)
> > 
> > Yes, that happens after parsing.
> 
> "%TRUE If" -> "%TRUE if"
> 
> > Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitSettings.cpp:437
> > +        " <body onload='document.title = \"JavaScript allowed from body onload attribute\"'>"
> 
> JavaScript improperly allowed
> 
> > Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitSettings.cpp:439
> > +        "  <script>document.title = 'JavaScript allowed from body script'</script>"
> 
> JavaScript improperly allowed
> 
> > Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitSettings.cpp:447
> > +    g_assert(jsResult);
> 
> g_assert_nonnull

-- 
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/20190121/0c79bd48/attachment.html>