Previsualisation broken

[KevinH]

It was set that way for "safety sake" but the user can disable all javascript if they so desire, so I would not ifdef it at all. Just straight change it to what Qt 5.15 needs and all other versions can live with it.

Thanks!

Kevin

[DiapDealer]

Originally Posted by KevinH

It was set that way for "safety sake" but the user can disable all javascript if they so desire, so I would not ifdef it at all. Just straight change it to what Qt 5.15 needs and all other versions can live with it.

Thanks!

Kevin

Works for me! I was hoping for a one-liner that I can get Arch to cherry-pick/patch to get Sigil functional again quickly. I don't want to have to do a release at this exact moment.

I've got a fix/refactor for my darkmode woes RE 5.15 as well, but that can wait a bit. it's cosmetic-only.

[KevinH]

BTW, this is a bug in Qt 5.15 added when they they adapted their code to a new chromium version. (It is in their adaption to Chromium 78). The original code only disabled access via the file url scheme (file:///) from within a web page but somehow NOW all local urls are now being translated to be equivalent to file: /// url schemes even if they do NOT explicitly use that url scheme.


So their bug fix changed/broke the meaning of the setting we used. So please do make this change permanent to prevent this nonsense.

[KevinH]

I used github "blame" on the qwebengine file in question and even though the change that is causing us issues has been in place for over 5 months, it was never included in any of the Qt 5.13.x, or 5.14.x releases.

Why on earth would you wait to release a change like this into a "LTS" release and never put it in any of the earlier dev series releases? It is almost like Qt wants to break things with this type of strategy. Or possibly it wants to hold out on "fixes" to so they are only available to commercial LTS customers first (or not at all!).

See https://github.com/qt/qtwebengine/bl...t_qt.cpp#L1147

And then check the 5.13, and 5.14 branches and see that these changes are not in them at all.

[kovidgoyal]

Just FYI turning that setting off means any book can now exfiltrate any of your user's data, as long as javascript is enabled. The correct way to fix this, is to define a custom URL scheme and use QWebEngineurlSchemeHandler to map it to local files in a secure manner. grep the calibre source code for QWebEngineUrlSchemeHandler for examples.

[KevinH]

Thanks for the tip, I will check it out. As you said, that attack mode would require the user to enable javascript which is under the user's control and have a malicious epub as well.

[KevinH]

Just checked it out. If I am following it correctly.

For a xhtml file locally loaded (no network access at all) to open its own co-located resources (images, css, etc) (again no network access) we would now have to now create our own fake scheme "sigl" and remap those local requests to our fake url scheme just to get them to be allowed, but only for Qt 5.15 and just to screen out file:/// url schemes.

Basically we are recreating our own webengine setting that prevented this in the first place because they broke something yet again.

That is absurd.

[KevinH]

Perhaps, it would be easier to set it to its default of true and then screen out any local file access in AcceptNavigationRequest that is outside the target epub.

I really do not want to have to register my own url schemes just to filter out file:/// scheme urls from within a evil epub.

I will see if that is a workable approach. If not, we will have to go with our own custom Scheme handler as calibre does.

[kovidgoyal]

The idea is you dont laod any files locally. Even the base HTML file you load via the custom scheme. Then all resources in the file are automatically also loaded via the custom scheme. And JS running from the custom scheme is not allowed access to local files anyway, without even needing the setting. All in all, its about a 100 lines of code. Remember to register your scheme as "trusted" with Qt so that it has the smae semantics as a page loaded over https and it can use html features gated behind https.

[KevinH]

We set the Local Storage preference setting to true so that javascript can store values it might want to cache before cleaning it up on close. Would I need to do something similar to allow that as well?

[KevinH]

And what is to keep the evil epub developer from simply using a "sigl" or calibre's FAKE_PROTOCOL scheme url to do what they want. Isn't this just moving the problem? It would be quite easy for a javascript to to get the current page url, find the newly trusted protocol/scheme and create a url using it, wouldn't it?

[KevinH]

Unless I am missing how using our own trusted protocol actually helps instead of moving the problem, perhaps it would be better to instead intercept it in acceptNavigationRequest and reject any file:/// url that is outside the bounds of the ebook folder. Javascript generated target links do pass through there and can be stopped at that point.

[KevinH]

Either way we would have to allow our local file:/// mathjax url, and local user css urls to be passed through.

[kovidgoyal]

Originally Posted by KevinH

And what is to keep the evil epub developer from simply using a "sigl" or calibre's FAKE_PROTOCOL scheme url to do what they want. Isn't this just moving the problem? It would be quite easy for a javascript to to get the current page url, find the newly trusted protocol/scheme and create a url using it, wouldn't it?

And in your implementation of the new protocol you deny access to all resources that are not part of the book.

[kovidgoyal]

Originally Posted by KevinH

Either way we would have to allow our local file:/// mathjax url, and local user css urls to be passed through.

No, you wouldn't. Those can also be run through the custom protocol.