Support for the <iframe> sandbox attribute

[kiwiz]

For my specific usecase, I'd like to enforce the existence of the sandbox attribute on <iframe>s. I haven't dug too deeply into the code, but it looks like patterns like these are usually special cased (ex: AddTargetBlankToFullyQualifiedLinks, RequireNoFollowOnLinks, etc).

Would a general purpose RequiredAttr method be considered for inclusion in the library? If so, it'd allow users to define such behaviours purely within a policy (which I think would be useful). Otherwise, I can work on a PR to just handle <iframe sandbox>.

[kiwiz]

Looks like this is a dupe of #12, although the last activity on that was in 2015 - lemme know if the thinking on that has changed.

[grafana-dee]

I didn't know of the sandbox attribute, but reading the docs https://developer.mozilla.org/en-US/docs/Web/HTML/Element/iframe#attr-sandbox shows that this would be an excellent addition to bluemonday.

I'm not sure I want to have the library scope creep from security sanitizer to HTML rewriter in a general purpose way, those are distinct things and there are good arguments to be made in favour of rewriting untrusted content pre-sanitization as well as rewriting the now trusted content post-sanitization... meaning a general purpose thing would be easy for people to mis-use and accidentally lower their security unintentionally.

But I am very much in favour of adding an option in the spirit of RequireNoFollowOnLinks, i.e. RequireSandboxOnIframe. I'm probably inclined to set up a range of constants for the different values and making that option require one of them, so that it might look like p.RequireSandboxOnIframe(bluemonday.SandboxDefaultEmpty) for the sandbox attribute alone and p.RequireSandboxOnIframe(bluemonday.SandboxAllowModals) for sandbox="allow-modals". Further that should probably consume a slice of such things as I see on https://www.html5rocks.com/en/tutorials/security/sandboxed-iframes/ that things like <iframe sandbox="allow-same-origin allow-scripts allow-popups allow-forms" src="https://platform.twitter.com/widgets/tweet_button.html" style="border: 0; width:130px; height:20px;"></iframe> are permitted.

How about something like this:

p.RequireSandboxOnIframe()
p.RequireSandboxOnIframe(SandboxAllowPopups, SandboxAllowPresentation)

And a sketch of that implemented would look like this https://go.dev/play/p/KKyYNeBlu6T :

package main

import (
	"fmt"
	"strings"
)

type SandboxValue int64

const (
	SandboxAllowDownloads SandboxValue = iota
	SandboxAllowDownloadsWithoutUserActivation
	SandboxAllowForms
	SandboxAllowModals
	SandboxAllowOrientationLock
	SandboxAllowPointerLock
	SandboxAllowPopups
	SandboxAllowPopupsToEscapeSandbox
	SandboxAllowPresentation
	SandboxAllowSameOrigin
	SandboxAllowScripts
	SandboxAllowStorageAccessByUserActivation
	SandboxAllowTopNavigation
	SandboxAllowTopNavigationByUserActivation
)

func requireSandboxOnIframe(vals ...SandboxValue) {
	if len(vals) == 0 {
		fmt.Println(`sandbox`)
		return
	}

	var renderedValues []string
	for _, val := range vals {
		switch val {
		case SandboxAllowDownloads:
			renderedValues = append(renderedValues, "allow-downloads")

		case SandboxAllowDownloadsWithoutUserActivation:
			renderedValues = append(renderedValues, "allow-downloads-without-user-activation")

		case SandboxAllowForms:
			renderedValues = append(renderedValues, "allow-forms")

		case SandboxAllowModals:
			renderedValues = append(renderedValues, "allow-modals")

		case SandboxAllowOrientationLock:
			renderedValues = append(renderedValues, "allow-orientation-lock")

		case SandboxAllowPointerLock:
			renderedValues = append(renderedValues, "allow-pointer-lock")

		case SandboxAllowPopups:
			renderedValues = append(renderedValues, "allow-popups")

		case SandboxAllowPopupsToEscapeSandbox:
			renderedValues = append(renderedValues, "allow-popups-to-escape-sandbox")

		case SandboxAllowPresentation:
			renderedValues = append(renderedValues, "allow-presentation")

		case SandboxAllowSameOrigin:
			renderedValues = append(renderedValues, "allow-same-origin")

		case SandboxAllowScripts:
			renderedValues = append(renderedValues, "allow-scripts")

		case SandboxAllowStorageAccessByUserActivation:
			renderedValues = append(renderedValues, "allow-storage-access-by-user-activation")

		case SandboxAllowTopNavigation:
			renderedValues = append(renderedValues, "allow-top-navigation")

		case SandboxAllowTopNavigationByUserActivation:
			renderedValues = append(renderedValues, "allow-top-navigation-by-user-activation")

		default:
		}
	}
	fmt.Println(`sandbox="` + strings.Join(renderedValues, ",") + `"`)
}

func main() {
	// Call it like this to get the default
	// Produces `sandbox`
	requireSandboxOnIframe()

	// Call it like this to declare which values you want for the sandbox attribute
	// Produces `sandbox="allow-popups,allow-presentation"`
	requireSandboxOnIframe(SandboxAllowPopups, SandboxAllowPresentation)
}

I've got a very hectic few weeks leading up to Christmas, but I will get to this by Christmas. If you want to beat me to it and produce a PR that implements the option along those lines and adjusts the sanitizer accordingly... please feel encouraged to do so, otherwise I will get to it eventually.

[grafana-dee]

Also... if you do take my example... I instantly reread it and nit'd on RequireSandboxOnIframe as it should probably be RequireSandboxOnIFrame. Whatever passes a linter works for me.

[kiwiz]

I'm not sure I want to have the library scope creep from security sanitizer to HTML rewriter

Yup, that sounds perfectly reasonable.

---

Going off that spirit, I think the list of values that's provided to RequireSandboxOnIFrame() should actually be an allowlist (since the way the sandbox attr works is that security is weakened by adding additional entries). So:

RequireSandboxOnIFrame() on:

• <iframe sandbox="allow-same-origin"> -> <iframe sandbox="">

RequireSandboxOnIFrame(SandboxAllowPopups) on:

• <iframe sandbox="allow-same-origin"> -> <iframe sandbox="">

If that sounds reasonable, I can take a stab at it (hopefully) sometime next week.

[grafana-dee]

That does sound reasonable... would merge 👍