dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
34.84k stars 9.84k forks source link

Blazor raises Permission Denied error when calling JS function with a cross-origin Window object as parameter #48521

Open datvm opened 1 year ago

datvm commented 1 year ago

Is there an existing issue for this?

Describe the bug

My app keeps an IJSObjectReference instance of a IWindow object obtained from window.opener. The opener is from another origin so most operation throws Permission Denied error and I only intend to use it for postMessage. However if I pass it to any JS function, Blazor attempts to call Object.prototype.hasOwnProperty on it and causes the app to crash. This also happens to iframes.

Blocked a frame with origin "https://localhost:44357" from accessing a cross-origin frame.

image

globalThis.wasmInterop = new class {

    getOpener() {
        return globalThis.parent;
    }

    sendReadyMessage(window, op) {
        window.postMessage({
            op,
        });
    }

}();
@inject IJSRuntime Js;
// ...

IJSObjectReference? opener;

protected override async Task OnInitializedAsync()
{
    opener = await Js.InvokeAsync<IJSObjectReference?>("wasmInterop.getOpener");
}

async Task SendMessageAsync()
{
    ArgumentNullException.ThrowIfNull(opener);

    // This one is fine
    await opener.InvokeVoidAsync("postMessage", "hello", "*");

    // This one raises the error
    await Js.InvokeVoidAsync("wasmInterop.sendReadyMessage", opener, "hello");
}

Interestingly, invoking the reference directly doesn't cause a problem. It only happens when it is a function parameter. I traced the source code back to this line in ElementReferenceCapture.ts:

DotNet.attachReviver((key, value) => {
  if (value && typeof value === 'object' && Object.prototype.hasOwnProperty.call(value, elementRefKey) && typeof value[elementRefKey] === 'string') {
    return getElementByCaptureId(value[elementRefKey]);
  } else {
    return value;
  }
});

Expected Behavior

Ideally hasOwnProperty should not be called, or at least Exception should be handled.

Steps To Reproduce

Github repo for the above code: https://github.com/datvm/BlazorHasOwnPropertyBugDemo, you should run both website, the bug can be seen by running the ASP.NET Core website on the Index page, click "Send message" button in the iframe.

Exceptions (if any)

No response

.NET Version

7.0.302

Anything else?

No response

datvm commented 1 year ago

Current workaround is to wrap the IWindow object inside another Javascript object.

I wonder if the assignment of __internalId to that object was even successful though I didn't notice anything in the console. However calling opener.InvokeVoidAsync was successful so I guess Blazor did somehow tracks it but didn't need to call hasOwnProperty there.

mkArtakMSFT commented 1 year ago

@javiercn what do you think about this? Can we avoid calling the hasOwnProperty API?

javiercn commented 1 year ago

@mkArtakMSFT It's likely a missing feature.

We would need a full repro to better understand the scenario and we can do about it. We are unlikely to have test coverage for edge cases like this.

We would potentially accept a PR if there is an easy fix, but otherwise, the solution of wrapping the object within a proxy seems appropriate.

ghost commented 1 year ago

Hi @datvm. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

datvm commented 1 year ago

@javiercn thanks for the response. I updated the Github repo and some information in the first post. You can now reproduce it consistently by using iframe (the ASP.NET project Index page has an iframe that open the Blazor project). You just need to run both projects at the same time to see the problem.

As for a PR, I would happy to give a fix, probably by wrapping it with a try-catch since the check that invokes hasOwnProperty is only for getting a DOM element (which doesn't apply for this Window object).

~IMO a more proper solution should be adding a check value instanceof Element instead~

Instead, a more proper solution should be adding a check like value instanceof Window. However this would only solve the problem for this specific case while a try-catch would solve potential problems we may have.

javiercn commented 1 year ago

@datvm let us think a bit about it.

For now, I think wrapping it into a proxy seems the right call to get unblocked. We'll put it in the backlog and give some additional thought to our options here.

ghost commented 1 year ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

ghost commented 7 months ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.