Should we handle `{ x-frame-options: SAMEORIGIN }` header ? #606

jjavierdguezas · 2021-06-16T16:48:38Z

jjavierdguezas
Jun 16, 2021
Collaborator

I found and interesting scenario related to <iframe/> tag and the {'x-frame-options', 'SAMEORIGIN'} header.

First, when that header is set, the resource can't be loaded in an iframe outside of its domain. So this iframe is not able to display cross domain.

The easiest way I tested this was using vscode live server and express.js:

index.js file with express server:

const express = require('express')
const app = express()
const port = 2000

app.get('/', (req, res) => {
  res.setHeader('x-frame-options', 'SAMEORIGIN'); // <-- We set the header here
  res.send('<div>Hello World!</div>')
})

app.listen(port, () => {
  console.log(`Example app listening at http://localhost:${port}`)
})

and the index.html file for Live Server

<!DOCTYPE html>
<html lang="en">

<head>
    <meta charset="UTF-8">
    <meta http-equiv="X-UA-Compatible" content="IE=edge">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Document</title>
</head>

<body>
    <iframe src="http://localhost:2000" title="description"></iframe>
</body>

</html>

if we run node index.js and open index.html file with live server, on chrome we see this:

but if we open index.html served by Live Server in Responsively we get an error

Should we take this in consideration for responsively?

If so, an easy solution found in electron/electron#426 (comment) that works well is to put the following in main.dev :

session.defaultSession.webRequest.onHeadersReceived(
    {urls: ['*://*/*']},
    (d, c) => {
      if (d.responseHeaders['X-Frame-Options']) {
        delete d.responseHeaders['X-Frame-Options'];
      } else if (d.responseHeaders['x-frame-options']) {
        delete d.responseHeaders['x-frame-options'];
      }

      c({cancel: false, responseHeaders: d.responseHeaders});
    }
  );

that intercepts requests and remove the x-frame-options header. That way the <iframe> loads just fine.

We could also make this a configuration like the Disable SSL Validation one and let the user decide this behavior.
It would be nice also, if we can display an image like chrome if the iframe can't be loaded instead of displaying a top-level error. That way we render the rest of the page and honor the website "wishes", but maybe that is more difficult and unnecessary

What do you think @manojVivek

jjavierdguezas · 2021-06-16T17:13:28Z

jjavierdguezas
Jun 16, 2021
Collaborator Author

Also we can just change in Webview/index.js:244 when the error is not from the main frame like this:

this.webviewRef.current.addEventListener(
      'did-fail-load',
-     ({errorCode, errorDescription}) => {
+     ({errorCode, errorDescription, isMainFrame}) => {
-        if (errorCode === -3) {
+        if (!isMainFrame || errorCode === -3) {
          // Aborted error, can be ignored
          return;
        }
        this.setState({
          errorCode,
          errorDesc: errorDescription,
        });
      }
    );

or even

this.webviewRef.current.addEventListener(
      'did-fail-load',
-     ({errorCode, errorDescription}) => {
+     ({errorCode, errorDescription, isMainFrame}) => {
-        if (errorCode === -3) {
+        if (errorCode === -3 || (!isMainFrame && errorCode === -27)) {
          // Aborted error, can be ignored
          return;
        }
        this.setState({
          errorCode,
          errorDesc: errorDescription,
        });
      }
    );

and then it will be rendere like this:

0 replies

manojVivek · 2021-06-24T04:18:32Z

manojVivek
Jun 24, 2021
Maintainer

@jjavierdguezas I think we shouldn't allow this, that will be a real security hole. I think this cannot be compared to Disable SSL Validation option.

Disabling SSL validation puts the accessing user at vulnerability and so if the user voluntarily accepts it then it is fine.
But in the case of X-Frame-Options, if we disable it that makes the accessed page vulnerable. The parent frame will then be allowed access to do stuff like injecting script into the iframe, reading the iframe content, etc, which the accessed page didn't agree to by setting that header.

I think this is more against the web standards and will open possibilities for phishing-type attacks using Responsively.

1 reply

jjavierdguezas Jun 24, 2021
Collaborator Author

I understand and agree with you @manojVivek . But should we do something about the error? If a page has an invalid iframe but the rest of the content is correct, should we show an error on the whole page (like now) or just not show the content of the iframe but render the rest of the page (like chrome does)?

vs

manojVivek · 2021-06-24T11:27:07Z

manojVivek
Jun 24, 2021
Maintainer

Yeah @jjavierdguezas, your solution in #606 (comment) sounds good. But will be good if it is possible to show some message inside the iframe box. What do you think?

1 reply

jjavierdguezas Jun 24, 2021
Collaborator Author

yeah, a message would be good. I'll find a way 👍🏻

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Should we handle `{ x-frame-options: SAMEORIGIN }` header ? #606

{{title}}

{{editor}}'s edit

{{editor}}'s edit

Replies: 3 comments 2 replies

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

Select a reply

Should we handle { x-frame-options: SAMEORIGIN } header ? #606

jjavierdguezas Jun 16, 2021 Collaborator

Replies: 3 comments · 2 replies

jjavierdguezas Jun 16, 2021 Collaborator Author

manojVivek Jun 24, 2021 Maintainer

jjavierdguezas Jun 24, 2021 Collaborator Author

manojVivek Jun 24, 2021 Maintainer

jjavierdguezas Jun 24, 2021 Collaborator Author

Should we handle `{ x-frame-options: SAMEORIGIN }` header ? #606

jjavierdguezas
Jun 16, 2021
Collaborator

Replies: 3 comments 2 replies

jjavierdguezas
Jun 16, 2021
Collaborator Author

manojVivek
Jun 24, 2021
Maintainer

jjavierdguezas Jun 24, 2021
Collaborator Author

manojVivek
Jun 24, 2021
Maintainer

jjavierdguezas Jun 24, 2021
Collaborator Author