Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a warning sentence for iframe load event step #8022

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

negibokken
Copy link

@negibokken negibokken commented Jun 19, 2022

The attackers can use the presence or absence of an iframe load event on
the parent's iframe to guess the URL. And if the URL contains sensitive
information like a username, the attacker can steal that information.
The related information can be available on [1].

[1] https://crbug.com/1248444

(See WHATWG Working Mode: Changes for more details.)


/iframe-embed-object.html ( diff )

The attackers can use the presence or absence of an iframe load event on
the parent's iframe to guess the URL. And if the URL contains sensitive
information like a username, the attacker can steal that information.
The related information can be available on [1].

[1] https://crbug.com/1248444
@domenic
Copy link
Member

domenic commented Jun 19, 2022

This isn't the correct fix for the issue. Adding a warning with a "should" does not actually change the processing model; instead, you need to change the processing model to track any cross-origin navigations, and either fire or not-fire the load event.

Basically, you need to port https://chromium.googlesource.com/chromium/src/+/5f8ddef5a678553c9bb1491cd5710788e6d571b3%5E%21/#F0 into spec text.

@negibokken
Copy link
Author

@domenic Thank you for your review! Does it mean making more detailed step 5 of iframe load event steps like below? (I will clear the sentence for the spec later)

  1. Fire an event named load at element according to the following steps
    • If this same-document navigation was initiated by a cross-origin iframe
      and is cross-origin to its parent, fire onload on the owner iframe.
      • warning: Normally, the owner iframe's onload fires if and only if the window's
        onload fires (i.e., when a navigation to a different document completes).
        However, a cross-origin initiator can use the presence or absence of a
        load event to detect whether the navigation was same- or cross-document,
        and can therefore try to guess the url of a cross-origin iframe. Fire the
        iframe's onload to prevent this technique.
    • If ~~~ (describe the other cases and specify fire or non-fire the load event)

If so, the cases are the following 4 cases?

  • same-document navigation
    • initiated by a cross-origin iframe and cross-origin iframe to its parent -> fire (this case)
    • initiated by a same-origin iframe and same-origin iframe to its parent -> fire
  • cross-document navigation
    • initiated by a cross-origin iframe and cross-origin iframe to its parent -> ? (Need to research later)
    • initiated by a same-origin iframe and same-origin iframe to its parent -> ? (Need to research later)

@domenic
Copy link
Member

domenic commented Jun 20, 2022

I don't really understand your suggestion. Step 5 of the iframe load event steps currently always fires the event. So you're not actually changing anything by saying that it would fire. Maybe the iframe load event steps need to be called more often, instead.

Another, probably harder part is defining "initiated by" rigorously. I can think of many different interpretations of those words so just using them without defining them isn't enough to give a specification something people could implement. But we can talk more about that when you identify where, exactly, you need to fire the load event, where it isn't being fired already.

@negibokken
Copy link
Author

Step 5 of the iframe load event steps currently always fires the event.
Maybe the iframe load event steps need to be called more often, instead.

Ah, that's true. Sorry, I made a mistake. We may need to add the step to call fire iframe load event.

Another, probably harder part is defining "initiated by" rigorously. I can think of many different interpretations of those words so just using them without defining them isn't enough to give a specification something people could implement. But we can talk more about that when you identify where, exactly, you need to fire the load event, where it isn't being fired already.

I'll try to identify where we need to fire the load event and where it isn't being fired already.

@negibokken
Copy link
Author

Sorry for the late update. I’m still working on this. I'm thinking about where the load event firing process should be placed for the case of 1.

I think the following steps of the case of 1 are as below:

  1. Start from here, about updating iframe attribute.
  2. Go to the To process the iframe attributes, and in case 1, src of iframe is updated so we follow the step 2.
  3. Go to The shared attribute processing steps for iframe and frame elements, and run each step through step 8.
  4. Go to To navigate an iframe or frame, and run each step through step 4
  5. Go to To navigate

I'm currently checking the step of To navigate. Specifically step 7, how the browsingContext work when iframe.src is replaced. And I'm thinking it matches the condition of step 7.

If anything I find, I'll post the comment, and I'll update the PR.

Footnotes

  1. https://crbug.com/1248444 2 3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants