Skip to content

Commit 3be8d57

Browse files
committed
consolidate special attribute handling code in preparation for srcset support
1 parent eaa97f1 commit 3be8d57

1 file changed

Lines changed: 95 additions & 57 deletions

File tree

src/main/java/org/owasp/html/HtmlPolicyBuilder.java

Lines changed: 95 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -587,14 +587,56 @@ public HtmlPolicyBuilder withPostprocessor(HtmlStreamEventProcessor pp) {
587587
}
588588

589589
/**
590-
* Names of attributes from HTML 4 whose values are URLs.
591-
* Other attributes, e.g. <code>style</code> may contain URLs even though
592-
* there values are not URLs.
590+
* Maps attribute names that need extra handling to producers of those
591+
* extra guards.
593592
*/
594-
private static final Set<String> URL_ATTRIBUTE_NAMES = ImmutableSet.of(
595-
"action", "archive", "background", "cite", "classid", "codebase", "data",
596-
"dsync", "formaction", "href", "icon", "longdesc", "manifest", "poster",
597-
"profile", "src", "srcset", "usemap");
593+
private static final Map<String, AttributeGuardMaker> ATTRIBUTE_GUARDS;
594+
static {
595+
// For each URL attribute that is allowed, we further constrain it by
596+
// only allowing the value through if it specifies no protocol, or if it
597+
// specifies one in the allowedProtocols white-list.
598+
// This is done regardless of whether any protocols have been allowed, so
599+
// allowing the attribute "href" globally with the identity policy but
600+
// not white-listing any protocols, effectively disallows the "href"
601+
// attribute globally.
602+
ImmutableMap.Builder<String, AttributeGuardMaker> b =
603+
ImmutableMap.builder();
604+
AttributeGuardMaker identityGuard = new AttributeGuardMaker() {
605+
@Override
606+
AttributePolicy makeGuard(AttributeGuardIntermediates intermediates) {
607+
return intermediates.urlAttributePolicy;
608+
}
609+
};
610+
for (String urlAttributeName : new String[] {
611+
"action", "archive", "background", "cite", "classid", "codebase", "data",
612+
"dsync", "formaction", "href", "icon", "longdesc", "manifest", "poster",
613+
"profile", "src", "srcset", "usemap",
614+
}) {
615+
b.put(urlAttributeName, identityGuard);
616+
}
617+
b.put("style", new AttributeGuardMaker() {
618+
619+
@Override
620+
AttributePolicy makeGuard(AttributeGuardIntermediates intermediates) {
621+
if (intermediates.cssSchema == null) {
622+
return null;
623+
}
624+
final AttributePolicy styleUrlPolicyFinal = AttributePolicy.Util.join(
625+
intermediates.styleUrlPolicy, intermediates.urlAttributePolicy);
626+
return new StylingPolicy(
627+
intermediates.cssSchema,
628+
new Function<String, String>() {
629+
public String apply(String url) {
630+
return styleUrlPolicyFinal.apply(
631+
"img", "src",
632+
url != null ? url : "about:invalid");
633+
}
634+
});
635+
}
636+
637+
});
638+
ATTRIBUTE_GUARDS = b.build();
639+
}
598640

599641
/**
600642
* Produces a policy based on the allow and disallow calls previously made.
@@ -702,15 +744,7 @@ private CompiledState compilePolicies() {
702744
}
703745
}
704746

705-
// Implement protocol policies.
706-
// For each URL attribute that is allowed, we further constrain it by
707-
// only allowing the value through if it specifies no protocol, or if it
708-
// specifies one in the allowedProtocols white-list.
709-
// This is done regardless of whether any protocols have been allowed, so
710-
// allowing the attribute "href" globally with the identity policy but
711-
// not white-listing any protocols, effectively disallows the "href"
712-
// attribute globally.
713-
StylingPolicy stylingPolicy = null;
747+
// Add guards on top of any custom policies.
714748
{
715749
final AttributePolicy urlAttributePolicy;
716750
if (allowedProtocols.size() == 3
@@ -723,48 +757,33 @@ private CompiledState compilePolicies() {
723757
allowedProtocols);
724758
}
725759

726-
if (this.stylingPolicySchema != null) {
727-
final AttributePolicy styleUrlPolicyFinal = AttributePolicy.Util.join(
728-
styleUrlPolicy, urlAttributePolicy);
729-
stylingPolicy = new StylingPolicy(
730-
stylingPolicySchema,
731-
new Function<String, String>() {
732-
public String apply(String url) {
733-
return styleUrlPolicyFinal.apply(
734-
"img", "src",
735-
url != null ? url : "about:invalid");
736-
}
737-
});
738-
}
739-
740-
Set<String> toGuard = Sets.newLinkedHashSet(URL_ATTRIBUTE_NAMES);
741-
for (String urlAttributeName : URL_ATTRIBUTE_NAMES) {
742-
if (globalAttrPolicies.containsKey(urlAttributeName)) {
743-
toGuard.remove(urlAttributeName);
744-
globalAttrPolicies.put(urlAttributeName, AttributePolicy.Util.join(
745-
urlAttributePolicy, globalAttrPolicies.get(urlAttributeName)));
760+
Set<String> toGuard = Sets.newLinkedHashSet(ATTRIBUTE_GUARDS.keySet());
761+
AttributeGuardIntermediates intermediates = new AttributeGuardIntermediates(
762+
urlAttributePolicy, this.styleUrlPolicy, this.stylingPolicySchema);
763+
for (Map.Entry<String, AttributeGuardMaker> e : ATTRIBUTE_GUARDS.entrySet()) {
764+
String attributeName = e.getKey();
765+
if (globalAttrPolicies.containsKey(attributeName)) {
766+
toGuard.remove(attributeName);
767+
AttributePolicy guard = e.getValue().makeGuard(intermediates);
768+
globalAttrPolicies.put(attributeName, AttributePolicy.Util.join(
769+
guard, globalAttrPolicies.get(attributeName)));
746770
}
747771
}
748772
// Implement guards not implemented on global policies in the per-element
749773
// policy maps.
750774
for (Map.Entry<String, Map<String, AttributePolicy>> e
751775
: attrPolicies.entrySet()) {
752776
Map<String, AttributePolicy> policies = e.getValue();
753-
for (String urlAttributeName : toGuard) {
754-
if (policies.containsKey(urlAttributeName)) {
755-
policies.put(urlAttributeName, AttributePolicy.Util.join(
756-
urlAttributePolicy, policies.get(urlAttributeName)));
777+
for (String attributeName : toGuard) {
778+
if (policies.containsKey(attributeName)) {
779+
AttributePolicy guard = ATTRIBUTE_GUARDS.get(attributeName)
780+
.makeGuard(intermediates);
781+
policies.put(attributeName, AttributePolicy.Util.join(
782+
guard, policies.get(attributeName)));
757783
}
758784
}
759785
}
760786
}
761-
if (stylingPolicy != null) {
762-
AttributePolicy old = globalAttrPolicies.get("style");
763-
if (old != null) {
764-
globalAttrPolicies.put(
765-
"style", AttributePolicy.Util.join(old, stylingPolicy));
766-
}
767-
}
768787

769788
ImmutableMap.Builder<String, ElementAndAttributePolicies> policiesBuilder
770789
= ImmutableMap.builder();
@@ -780,16 +799,6 @@ public String apply(String url) {
780799
if (elAttrPolicies == null) {
781800
elAttrPolicies = ImmutableMap.of();
782801
}
783-
if (stylingPolicy != null) {
784-
AttributePolicy old = elAttrPolicies.get("style");
785-
if (old != null) {
786-
attrPolicies.put(
787-
elementName,
788-
elAttrPolicies = Maps.newLinkedHashMap(elAttrPolicies));
789-
elAttrPolicies.put(
790-
"style", AttributePolicy.Util.join(old, stylingPolicy));
791-
}
792-
}
793802

794803
ImmutableMap.Builder<String, AttributePolicy> attrs
795804
= ImmutableMap.builder();
@@ -1073,3 +1082,32 @@ public JoinableElementPolicy join(
10731082
}
10741083
}
10751084

1085+
1086+
/**
1087+
* Given some policy builder state, produces a guard for an HTML attribute
1088+
* that needs extra handling on top of any policy-author-supplied custom
1089+
* attribute policy.
1090+
*/
1091+
abstract class AttributeGuardMaker {
1092+
abstract AttributePolicy makeGuard(AttributeGuardIntermediates intermediates);
1093+
}
1094+
1095+
/**
1096+
* A grab bag of state that is useful when coming up with guards for HTML
1097+
* attributes that need extra handling on top of any policy-author-supplied
1098+
* custom attribute policy.
1099+
*/
1100+
final class AttributeGuardIntermediates {
1101+
final AttributePolicy urlAttributePolicy;
1102+
final AttributePolicy styleUrlPolicy;
1103+
final CssSchema cssSchema;
1104+
1105+
AttributeGuardIntermediates(
1106+
AttributePolicy urlAttributePolicy,
1107+
AttributePolicy styleUrlPolicy,
1108+
CssSchema cssSchema) {
1109+
this.urlAttributePolicy = urlAttributePolicy;
1110+
this.styleUrlPolicy = styleUrlPolicy;
1111+
this.cssSchema = cssSchema;
1112+
}
1113+
}

0 commit comments

Comments
 (0)