Skip to content

Commit

Permalink
ArticleView: expose only minimal API to JavaScript
Browse files Browse the repository at this point in the history
https://doc.qt.io/archives/qt-5.5/qtwebkit-bridge.html#internet-security
Qt WebKit Bridge documentation recommends:
  When exposing native objects to an open web environment, it is
  important to understand the security implications. Think whether the
  exposed object enables the web environment access things that
  shouldn't be open, and whether the web content loaded by that web page
  comes from a trusted source.

The author of Qt WebChannel has said the following in a talk that
introduced this Qt module (WebKit Bridge replacement for Qt WebEngine):
  My suggestion here is to write dedicated QObjects with a slim, minimal
  API that only have the signals and methods that you deem safe to be
  used from the outside.
- see a comment under https://redirect.invidious.io/watch?v=KnvnTi6XafA
  • Loading branch information
vedgy committed Jun 2, 2022
1 parent aa33b8b commit 99982a1
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 1 deletion.
23 changes: 22 additions & 1 deletion articleview.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,24 @@
using std::map;
using std::list;

/// This class exposes only slim, minimal API to JavaScript clients in order to
/// reduce attack surface available to potentionally malicious external scripts.
class ArticleViewJsProxy: public QObject
{
Q_OBJECT
public:
/// Note: view becomes the parent of this proxy object.
explicit ArticleViewJsProxy( ArticleView & view ):
QObject( &view ), articleView( view )
{}

Q_INVOKABLE void onJsActiveArticleChanged( QString const & id )
{ articleView.onJsActiveArticleChanged( id ); }

private:
ArticleView & articleView;
};

/// AccentMarkHandler class
///
/// Remove accent marks from text
Expand Down Expand Up @@ -222,6 +240,7 @@ ArticleView::ArticleView( QWidget * parent, ArticleNetworkAccessManager & nm,
groups( groups_ ),
popupView( popupView_ ),
cfg( cfg_ ),
jsProxy( new ArticleViewJsProxy( *this ) ),
pasteAction( this ),
articleUpAction( this ),
articleDownAction( this ),
Expand Down Expand Up @@ -1140,7 +1159,7 @@ void ArticleView::linkHovered ( const QString & link, const QString & , const QS

void ArticleView::attachToJavaScript()
{
ui.definition->page()->mainFrame()->addToJavaScriptWindowObject( QString( "articleview" ), this );
ui.definition->page()->mainFrame()->addToJavaScriptWindowObject( "articleview", jsProxy );
}

void ArticleView::linkClicked( QUrl const & url_ )
Expand Down Expand Up @@ -3090,3 +3109,5 @@ void ResourceToSaveHandler::downloadFinished()
deleteLater();
}
}

#include "articleview.moc"
3 changes: 3 additions & 0 deletions articleview.hh
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "groupcombobox.hh"
#include "ui_articleview.h"

class ArticleViewJsProxy;
class ResourceToSaveHandler;

/// A widget with the web view tailored to view and handle articles -- it
Expand All @@ -32,6 +33,8 @@ class ArticleView: public QFrame

Ui::ArticleView ui;

ArticleViewJsProxy * const jsProxy;

QAction pasteAction, articleUpAction, articleDownAction,
goBackAction, goForwardAction, selectCurrentArticleAction,
copyAsTextAction, inspectAction;
Expand Down

0 comments on commit 99982a1

Please sign in to comment.