Issue #2736 - Part 7: Re-work <frame> and <iframe> src attribute.

Use subject principal as triggering principal in <iframe>/<frame> "src" attribute.
This commit is contained in:
Moonchild 2025-04-29 13:00:46 +02:00 committed by roytam1
commit 2416e71cd3
9 changed files with 82 additions and 28 deletions

View file

@ -57,6 +57,7 @@
#include "nsView.h"
#include "GroupedSHistory.h"
#include "PartialSHistory.h"
#include "nsQueryObject.h"
#include "nsIURI.h"
#include "nsIURL.h"
@ -86,6 +87,7 @@
#include "mozilla/dom/Element.h"
#include "mozilla/jsipc/CrossProcessObjectWrappers.h"
#include "mozilla/layout/RenderFrameParent.h"
#include "nsGenericHTMLFrameElement.h"
#include "GeckoProfiler.h"
#include "jsapi.h"
@ -231,6 +233,7 @@ nsFrameLoader::LoadFrame()
NS_ENSURE_TRUE(mOwnerContent, NS_ERROR_NOT_INITIALIZED);
nsAutoString src;
nsCOMPtr<nsIPrincipal> principal;
bool isSrcdoc = mOwnerContent->IsHTMLElement(nsGkAtoms::iframe) &&
mOwnerContent->HasAttr(kNameSpaceID_None, nsGkAtoms::srcdoc);
@ -238,7 +241,7 @@ nsFrameLoader::LoadFrame()
src.AssignLiteral("about:srcdoc");
}
else {
GetURL(src);
GetURL(src, getter_AddRefs(principal));
src.Trim(" \t\n\r");
@ -279,7 +282,7 @@ nsFrameLoader::LoadFrame()
}
if (NS_SUCCEEDED(rv)) {
rv = LoadURI(uri);
rv = LoadURI(uri, principal);
}
if (NS_FAILED(rv)) {
@ -305,7 +308,7 @@ nsFrameLoader::FireErrorEvent()
}
NS_IMETHODIMP
nsFrameLoader::LoadURI(nsIURI* aURI)
nsFrameLoader::LoadURI(nsIURI* aURI, nsIPrincipal* aTriggeringPrincipal)
{
if (!aURI)
return NS_ERROR_INVALID_POINTER;
@ -313,13 +316,15 @@ nsFrameLoader::LoadURI(nsIURI* aURI)
nsCOMPtr<nsIDocument> doc = mOwnerContent->OwnerDoc();
nsresult rv = CheckURILoad(aURI);
nsresult rv = CheckURILoad(aURI, aTriggeringPrincipal);
NS_ENSURE_SUCCESS(rv, rv);
mURIToLoad = aURI;
mTriggeringPrincipal = aTriggeringPrincipal ? aTriggeringPrincipal : nullptr;
rv = doc->InitializeFrameLoader(this);
if (NS_FAILED(rv)) {
mURIToLoad = nullptr;
mTriggeringPrincipal = nullptr;
}
return rv;
}
@ -513,7 +518,7 @@ nsFrameLoader::ReallyStartLoadingInternal()
"MaybeCreateDocShell succeeded with a null mDocShell");
// Just to be safe, recheck uri.
rv = CheckURILoad(mURIToLoad);
rv = CheckURILoad(mURIToLoad, mTriggeringPrincipal);
NS_ENSURE_SUCCESS(rv, rv);
nsCOMPtr<nsIDocShellLoadInfo> loadInfo;
@ -527,7 +532,11 @@ nsFrameLoader::ReallyStartLoadingInternal()
// We'll use our principal, not that of the document loaded inside us. This
// is very important; needed to prevent XSS attacks on documents loaded in
// subframes!
loadInfo->SetTriggeringPrincipal(mOwnerContent->NodePrincipal());
if (mTriggeringPrincipal) {
loadInfo->SetTriggeringPrincipal(mTriggeringPrincipal);
} else {
loadInfo->SetTriggeringPrincipal(mOwnerContent->NodePrincipal());
}
nsCOMPtr<nsIURI> referrer;
@ -601,7 +610,7 @@ nsFrameLoader::ReallyStartLoadingInternal()
}
nsresult
nsFrameLoader::CheckURILoad(nsIURI* aURI)
nsFrameLoader::CheckURILoad(nsIURI* aURI, nsIPrincipal* aTriggeringPrincipal)
{
// Check for security. The fun part is trying to figure out what principals
// to use. The way I figure it, if we're doing a LoadFrame() accidentally
@ -620,7 +629,9 @@ nsFrameLoader::CheckURILoad(nsIURI* aURI)
nsIScriptSecurityManager *secMan = nsContentUtils::GetSecurityManager();
// Get our principal
nsIPrincipal* principal = mOwnerContent->NodePrincipal();
nsIPrincipal* principal = (aTriggeringPrincipal ?
aTriggeringPrincipal :
mOwnerContent->NodePrincipal());
// Check if we are allowed to load absURL
nsresult rv =
@ -2191,7 +2202,7 @@ nsFrameLoader::MaybeCreateDocShell()
}
void
nsFrameLoader::GetURL(nsString& aURI)
nsFrameLoader::GetURL(nsString& aURI, nsIPrincipal** aTriggeringPrincipal)
{
aURI.Truncate();
@ -2199,6 +2210,10 @@ nsFrameLoader::GetURL(nsString& aURI)
mOwnerContent->GetAttr(kNameSpaceID_None, nsGkAtoms::data, aURI);
} else {
mOwnerContent->GetAttr(kNameSpaceID_None, nsGkAtoms::src, aURI);
if (RefPtr<nsGenericHTMLFrameElement> frame = do_QueryObject(mOwnerContent)) {
nsCOMPtr<nsIPrincipal> prin = frame->GetSrcTriggeringPrincipal();
prin.forget(aTriggeringPrincipal);
}
}
}

View file

@ -215,7 +215,7 @@ public:
*/
void ApplySandboxFlags(uint32_t sandboxFlags);
void GetURL(nsString& aURL);
void GetURL(nsString& aURL, nsIPrincipal** aTriggeringPrincipal);
// Properly retrieves documentSize of any subdocument type.
nsresult GetWindowDimensions(nsIntRect& aRect);
@ -264,7 +264,17 @@ private:
// Updates the subdocument position and size. This gets called only
// when we have our own in-process DocShell.
void UpdateBaseWindowPositionAndSize(nsSubDocumentFrame *aIFrame);
nsresult CheckURILoad(nsIURI* aURI);
/**
* Checks whether a load of the given URI should be allowed, and returns an
* error result if it should not.
*
* @param aURI The URI to check.
* @param aTriggeringPrincipal The triggering principal for the load. May be
* null, in which case the node principal of the owner content is used.
*/
nsresult CheckURILoad(nsIURI* aURI, nsIPrincipal* aTriggeringPrincipal);
void FireErrorEvent();
nsresult ReallyStartLoadingInternal();
@ -302,6 +312,7 @@ private:
nsCOMPtr<nsIDocShell> mDocShell;
nsCOMPtr<nsIURI> mURIToLoad;
nsCOMPtr<nsIPrincipal> mTriggeringPrincipal;
mozilla::dom::Element* mOwnerContent; // WEAK
// After the frameloader has been removed from the DOM but before all of the

View file

@ -8,6 +8,7 @@
interface nsFrameLoader;
interface nsIDocShell;
interface nsIURI;
interface nsIPrincipal;
interface nsIFrame;
interface nsSubDocumentFrame;
interface nsIMessageSender;
@ -51,7 +52,7 @@ interface nsIFrameLoader : nsISupports
* Loads the specified URI in this frame. Behaves identically to loadFrame,
* except that this method allows specifying the URI to load.
*/
void loadURI(in nsIURI aURI);
void loadURI(in nsIURI aURI, [optional] in nsIPrincipal aTriggeringPrincipal);
/**
* Puts the frameloader in prerendering mode.

View file

@ -82,10 +82,13 @@ public:
SetHTMLAttr(nsGkAtoms::scrolling, aScrolling, aError);
}
// The XPCOM GetSrc is OK for us
void SetSrc(const nsAString& aSrc, ErrorResult& aError)
void GetSrc(nsString& aSrc, nsIPrincipal&)
{
SetAttrHelper(nsGkAtoms::src, aSrc);
GetURIAttr(nsGkAtoms::src, nullptr, aSrc);
}
void SetSrc(const nsAString& aSrc, nsIPrincipal& aTriggeringPrincipal, ErrorResult& aError)
{
SetHTMLAttr(nsGkAtoms::src, aSrc, aTriggeringPrincipal, aError);
}
using nsGenericHTMLFrameElement::GetContentDocument;

View file

@ -48,10 +48,13 @@ public:
uint32_t GetSandboxFlags();
// Web IDL binding methods
// The XPCOM GetSrc is fine for our purposes
void SetSrc(const nsAString& aSrc, ErrorResult& aError)
void GetSrc(nsString& aSrc, nsIPrincipal&) const
{
SetHTMLAttr(nsGkAtoms::src, aSrc, aError);
GetURIAttr(nsGkAtoms::src, nullptr, aSrc);
}
void SetSrc(const nsAString& aSrc, nsIPrincipal& aTriggeringPrincipal, ErrorResult& aError)
{
SetHTMLAttr(nsGkAtoms::src, aSrc, aTriggeringPrincipal, aError);
}
void GetSrcdoc(DOMString& aSrcdoc)
{

View file

@ -338,14 +338,14 @@ PrincipalAllowsBrowserFrame(nsIPrincipal* aPrincipal)
nsGenericHTMLFrameElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
const nsAttrValue* aValue,
const nsAttrValue* aOldValue,
nsIPrincipal* aSubjectPrincipal,
nsIPrincipal* aMaybeScriptedPrincipal,
bool aNotify)
{
if (aValue) {
nsAttrValueOrString value(aValue);
AfterMaybeChangeAttr(aNameSpaceID, aName, &value, aNotify);
AfterMaybeChangeAttr(aNameSpaceID, aName, &value, aMaybeScriptedPrincipal, aNotify);
} else {
AfterMaybeChangeAttr(aNameSpaceID, aName, nullptr, aNotify);
AfterMaybeChangeAttr(aNameSpaceID, aName, nullptr, aMaybeScriptedPrincipal, aNotify);
}
if (aNameSpaceID == kNameSpaceID_None) {
@ -378,7 +378,7 @@ nsGenericHTMLFrameElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName,
}
return nsGenericHTMLElement::AfterSetAttr(aNameSpaceID, aName, aValue,
aOldValue, aSubjectPrincipal,
aOldValue, aMaybeScriptedPrincipal,
aNotify);
}
@ -388,20 +388,23 @@ nsGenericHTMLFrameElement::OnAttrSetButNotChanged(int32_t aNamespaceID,
const nsAttrValueOrString& aValue,
bool aNotify)
{
AfterMaybeChangeAttr(aNamespaceID, aName, &aValue, aNotify);
AfterMaybeChangeAttr(aNamespaceID, aName, &aValue, nullptr, aNotify);
return nsGenericHTMLElement::OnAttrSetButNotChanged(aNamespaceID, aName,
aValue, aNotify);
}
void
nsGenericHTMLFrameElement::AfterMaybeChangeAttr(int32_t aNamespaceID,
nsGenericHTMLFrameElement::AfterMaybeChangeAttr(int32_t aNameSpaceID,
nsIAtom* aName,
const nsAttrValueOrString* aValue,
nsIPrincipal* aMaybeScriptedPrincipal,
bool aNotify)
{
if (aNamespaceID == kNameSpaceID_None) {
if (aNameSpaceID == kNameSpaceID_None) {
if (aName == nsGkAtoms::src) {
mSrcTriggeringPrincipal = nsContentUtils::GetAttrTriggeringPrincipal(
this, aValue ? aValue->String() : EmptyString(), aMaybeScriptedPrincipal);
if (aValue && (!IsHTMLElement(nsGkAtoms::iframe) ||
!HasAttr(kNameSpaceID_None, nsGkAtoms::srcdoc))) {
// Don't propagate error here. The attribute was successfully set,

View file

@ -18,6 +18,10 @@
class nsXULElement;
#define NS_GENERICHTMLFRAMEELEMENT_IID \
{ 0x8190db72, 0xdab0, 0x4d72, \
{ 0x94, 0x26, 0x87, 0x5f, 0x5a, 0x8a, 0x2a, 0xe5 } }
/**
* A helper class for frame elements
*/
@ -45,6 +49,8 @@ public:
NS_DECL_NSIDOMMOZBROWSERFRAME
NS_DECL_NSIMOZBROWSERFRAME
NS_DECLARE_STATIC_IID_ACCESSOR(NS_GENERICHTMLFRAMEELEMENT_IID)
// nsIContent
virtual bool IsHTMLFocusable(bool aWithMouse, bool *aIsFocusable, int32_t *aTabIndex) override;
virtual nsresult BindToTree(nsIDocument* aDocument, nsIContent* aParent,
@ -86,6 +92,11 @@ public:
*/
static int32_t MapScrollingAttribute(const nsAttrValue* aValue);
nsIPrincipal* GetSrcTriggeringPrincipal() const
{
return mSrcTriggeringPrincipal;
}
protected:
virtual ~nsGenericHTMLFrameElement();
@ -109,6 +120,8 @@ protected:
RefPtr<nsFrameLoader> mFrameLoader;
nsCOMPtr<nsPIDOMWindowOuter> mOpenerWindow;
nsCOMPtr<nsIPrincipal> mSrcTriggeringPrincipal;
/**
* True when the element is created by the parser using the
* NS_FROM_PARSER_NETWORK flag.
@ -139,7 +152,12 @@ private:
* @param aNotify Whether we plan to notify document observers.
*/
void AfterMaybeChangeAttr(int32_t aNamespaceID, nsIAtom* aName,
const nsAttrValueOrString* aValue, bool aNotify);
const nsAttrValueOrString* aValue,
nsIPrincipal* aMaybeScriptedPrincipal,
bool aNotify);
};
NS_DEFINE_STATIC_IID_ACCESSOR(nsGenericHTMLFrameElement,
NS_GENERICHTMLFRAMEELEMENT_IID)
#endif // nsGenericHTMLFrameElement_h

View file

@ -17,7 +17,7 @@ interface HTMLFrameElement : HTMLElement {
attribute DOMString name;
[CEReactions, SetterThrows]
attribute DOMString scrolling;
[CEReactions, SetterThrows]
[CEReactions, NeedsSubjectPrincipal, SetterThrows]
attribute DOMString src;
[CEReactions, SetterThrows]
attribute DOMString frameBorder;

View file

@ -13,7 +13,7 @@
[HTMLConstructor]
interface HTMLIFrameElement : HTMLElement {
[CEReactions, SetterThrows, Pure]
[CEReactions, NeedsSubjectPrincipal, SetterThrows, Pure]
attribute DOMString src;
[CEReactions, SetterThrows, Pure]
attribute DOMString srcdoc;