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

Fix/safelist deep copy constructor #1763

Merged
merged 3 commits into from May 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 18 additions & 8 deletions src/main/java/org/jsoup/safety/Safelist.java
Expand Up @@ -63,10 +63,10 @@ XSS attack examples (that jsoup will safegaurd against the default Cleaner and S
</p>
*/
public class Safelist {
private Set<TagName> tagNames; // tags allowed, lower case. e.g. [p, br, span]
private Map<TagName, Set<AttributeKey>> attributes; // tag -> attribute[]. allowed attributes [href] for a tag.
private Map<TagName, Map<AttributeKey, AttributeValue>> enforcedAttributes; // always set these attribute values
private Map<TagName, Map<AttributeKey, Set<Protocol>>> protocols; // allowed URL protocols for attributes
private final Set<TagName> tagNames; // tags allowed, lower case. e.g. [p, br, span]
private final Map<TagName, Set<AttributeKey>> attributes; // tag -> attribute[]. allowed attributes [href] for a tag.
private final Map<TagName, Map<AttributeKey, AttributeValue>> enforcedAttributes; // always set these attribute values
private final Map<TagName, Map<AttributeKey, Set<Protocol>>> protocols; // allowed URL protocols for attributes
private boolean preserveRelativeLinks; // option to preserve relative links

/**
Expand Down Expand Up @@ -203,9 +203,19 @@ public Safelist() {
public Safelist(Safelist copy) {
this();
tagNames.addAll(copy.tagNames);
attributes.putAll(copy.attributes);
enforcedAttributes.putAll(copy.enforcedAttributes);
protocols.putAll(copy.protocols);
for (Map.Entry<TagName, Set<AttributeKey>> copyTagAttributes : copy.attributes.entrySet()) {
attributes.put(copyTagAttributes.getKey(), new HashSet<>(copyTagAttributes.getValue()));
}
for (Map.Entry<TagName, Map<AttributeKey, AttributeValue>> enforcedEntry : copy.enforcedAttributes.entrySet()) {
enforcedAttributes.put(enforcedEntry.getKey(), new HashMap<>(enforcedEntry.getValue()));
}
for (Map.Entry<TagName, Map<AttributeKey, Set<Protocol>>> protocolsEntry : copy.protocols.entrySet()) {
Map<AttributeKey, Set<Protocol>> attributeProtocolsCopy = new HashMap<>();
for (Map.Entry<AttributeKey, Set<Protocol>> attributeProtocols : protocolsEntry.getValue().entrySet()) {
attributeProtocolsCopy.put(attributeProtocols.getKey(), new HashSet<>(attributeProtocols.getValue()));
}
protocols.put(protocolsEntry.getKey(), attributeProtocolsCopy);
}
preserveRelativeLinks = copy.preserveRelativeLinks;
}

Expand Down Expand Up @@ -620,7 +630,7 @@ static Protocol valueOf(String value) {
}

abstract static class TypedValue {
private String value;
private final String value;

TypedValue(String value) {
Validate.notNull(value);
Expand Down
65 changes: 65 additions & 0 deletions src/test/java/org/jsoup/safety/SafelistTest.java
@@ -0,0 +1,65 @@
package org.jsoup.safety;

import org.jsoup.nodes.Attribute;
import org.jsoup.nodes.Attributes;
import org.jsoup.nodes.Element;
import org.jsoup.parser.Tag;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotEquals;

public class SafelistTest {
private static final String TEST_TAG = "testTag";
private static final String TEST_ATTRIBUTE = "testAttribute";
private static final String TEST_SCHEME = "valid-scheme";
private static final String TEST_VALUE = TEST_SCHEME + "://testValue";

@Test
public void testCopyConstructor_noSideEffectOnTags() {
Safelist safelist1 = Safelist.none().addTags(TEST_TAG);
Safelist safelist2 = new Safelist(safelist1);
safelist1.addTags("invalidTag");

assertFalse(safelist2.isSafeTag("invalidTag"));
}

@Test
public void testCopyConstructor_noSideEffectOnAttributes() {
Safelist safelist1 = Safelist.none().addAttributes(TEST_TAG, TEST_ATTRIBUTE);
Safelist safelist2 = new Safelist(safelist1);
safelist1.addAttributes(TEST_TAG, "invalidAttribute");

assertFalse(safelist2.isSafeAttribute(TEST_TAG, null, new Attribute("invalidAttribute", TEST_VALUE)));
}

@Test
public void testCopyConstructor_noSideEffectOnEnforcedAttributes() {
Safelist safelist1 = Safelist.none().addEnforcedAttribute(TEST_TAG, TEST_ATTRIBUTE, TEST_VALUE);
Safelist safelist2 = new Safelist(safelist1);
safelist1.addEnforcedAttribute(TEST_TAG, TEST_ATTRIBUTE, "invalidValue");

for (Attribute enforcedAttribute : safelist2.getEnforcedAttributes(TEST_TAG)) {
assertNotEquals("invalidValue", enforcedAttribute.getValue());
}
}

@Test
public void testCopyConstructor_noSideEffectOnProtocols() {
final String invalidScheme = "invalid-scheme";
Safelist safelist1 = Safelist.none()
.addAttributes(TEST_TAG, TEST_ATTRIBUTE)
.addProtocols(TEST_TAG, TEST_ATTRIBUTE, TEST_SCHEME);
Safelist safelist2 = new Safelist(safelist1);
safelist1.addProtocols(TEST_TAG, TEST_ATTRIBUTE, invalidScheme);

Attributes attributes = new Attributes();
Attribute invalidAttribute = new Attribute(TEST_ATTRIBUTE, invalidScheme + "://someValue");
attributes.put(invalidAttribute);
Element invalidElement = new Element(Tag.valueOf(TEST_TAG), "", attributes);

assertFalse(safelist2.isSafeAttribute(TEST_TAG, invalidElement, invalidAttribute));
}


}