Repository: wicket Updated Branches: refs/heads/wicket-6.x 9f6c69f8f -> e213b0d2c
Escape the generated markup for attribute names and values in CheckBoxMultipleChoice and RadioChoice Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/e213b0d2 Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/e213b0d2 Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/e213b0d2 Branch: refs/heads/wicket-6.x Commit: e213b0d2cd3b5d1876c4e6fee15750e2491063b6 Parents: 9f6c69f Author: Martin Tzvetanov Grigorov <[email protected]> Authored: Mon Dec 21 16:31:55 2015 +0100 Committer: Martin Tzvetanov Grigorov <[email protected]> Committed: Mon Dec 21 16:33:40 2015 +0100 ---------------------------------------------------------------------- .../html/form/CheckBoxMultipleChoice.java | 12 ++--- .../wicket/markup/html/form/RadioChoice.java | 18 +++---- .../form/EscapeAttributesInChoicesPage.html | 20 ++++++++ .../form/EscapeAttributesInChoicesPage.java | 53 ++++++++++++++++++++ .../form/EscapeAttributesInChoicesTest.java | 53 ++++++++++++++++++++ 5 files changed, 141 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/e213b0d2/wicket-core/src/main/java/org/apache/wicket/markup/html/form/CheckBoxMultipleChoice.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/CheckBoxMultipleChoice.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/CheckBoxMultipleChoice.java index 74d4e14..3563779 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/CheckBoxMultipleChoice.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/CheckBoxMultipleChoice.java @@ -424,7 +424,7 @@ public class CheckBoxMultipleChoice<T> extends ListMultipleChoice<T> { case BEFORE: buffer.append("<label for=\""); - buffer.append(idAttr); + buffer.append(Strings.escapeMarkup(idAttr)); buffer.append("\">").append(escaped).append("</label>"); break; case WRAP_AFTER: @@ -450,9 +450,9 @@ public class CheckBoxMultipleChoice<T> extends ListMultipleChoice<T> buffer.append(" disabled=\"disabled\""); } buffer.append(" value=\""); - buffer.append(id); + buffer.append(Strings.escapeMarkup(id)); buffer.append("\" id=\""); - buffer.append(idAttr); + buffer.append(Strings.escapeMarkup(idAttr)); buffer.append('"'); // Allows user to add attributes to the <input..> tag @@ -463,9 +463,9 @@ public class CheckBoxMultipleChoice<T> extends ListMultipleChoice<T> for (Map.Entry<String, Object> attr : attrs.entrySet()) { buffer.append(' ') - .append(attr.getKey()) + .append(Strings.escapeMarkup(attr.getKey())) .append("=\"") - .append(attr.getValue()) + .append(Strings.escapeMarkup(attr.getValue().toString())) .append('"'); } } @@ -495,7 +495,7 @@ public class CheckBoxMultipleChoice<T> extends ListMultipleChoice<T> break; case AFTER: buffer.append("<label for=\""); - buffer.append(idAttr); + buffer.append(Strings.escapeMarkup(idAttr)); buffer.append("\">").append(escaped).append("</label>"); break; } http://git-wip-us.apache.org/repos/asf/wicket/blob/e213b0d2/wicket-core/src/main/java/org/apache/wicket/markup/html/form/RadioChoice.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/RadioChoice.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/RadioChoice.java index 1218065..309fd59 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/RadioChoice.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/RadioChoice.java @@ -475,9 +475,9 @@ public class RadioChoice<T> extends AbstractSingleSelectChoice<T> implements IOn for (Map.Entry<String, Object> attr : labelAttrs.entrySet()) { extraLabelAttributes.append(' ') - .append(attr.getKey()) + .append(Strings.escapeMarkup(attr.getKey())) .append("=\"") - .append(attr.getValue()) + .append(Strings.escapeMarkup(attr.getValue().toString())) .append('"'); } } @@ -487,7 +487,7 @@ public class RadioChoice<T> extends AbstractSingleSelectChoice<T> implements IOn case BEFORE: buffer.append("<label for=\"") - .append(idAttr) + .append(Strings.escapeMarkup(idAttr)) .append('"') .append(extraLabelAttributes) .append('>') @@ -516,9 +516,9 @@ public class RadioChoice<T> extends AbstractSingleSelectChoice<T> implements IOn .append((isSelected(choice, index, selected) ? " checked=\"checked\"" : "")) .append((enabled ? "" : " disabled=\"disabled\"")) .append(" value=\"") - .append(id) + .append(Strings.escapeMarkup(id)) .append("\" id=\"") - .append(idAttr) + .append(Strings.escapeMarkup(idAttr)) .append('"'); // Should a roundtrip be made (have onSelectionChanged called) @@ -542,7 +542,7 @@ public class RadioChoice<T> extends AbstractSingleSelectChoice<T> implements IOn .append(url) .append((url.toString().indexOf('?') > -1 ? '&' : '?') + getInputName()) .append('=') - .append(id) + .append(Strings.escapeMarkup(id)) .append("';\""); } } @@ -555,9 +555,9 @@ public class RadioChoice<T> extends AbstractSingleSelectChoice<T> implements IOn for (Map.Entry<String, Object> attr : attrs.entrySet()) { buffer.append(' ') - .append(attr.getKey()) + .append(Strings.escapeMarkup(attr.getKey())) .append("=\"") - .append(attr.getValue()) + .append(Strings.escapeMarkup(attr.getValue().toString())) .append('"'); } } @@ -581,7 +581,7 @@ public class RadioChoice<T> extends AbstractSingleSelectChoice<T> implements IOn { case AFTER: buffer.append("<label for=\"") - .append(idAttr) + .append(Strings.escapeMarkup(idAttr)) .append('"') .append(extraLabelAttributes) .append('>') http://git-wip-us.apache.org/repos/asf/wicket/blob/e213b0d2/wicket-core/src/test/java/org/apache/wicket/markup/html/form/EscapeAttributesInChoicesPage.html ---------------------------------------------------------------------- diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/form/EscapeAttributesInChoicesPage.html b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/EscapeAttributesInChoicesPage.html new file mode 100644 index 0000000..2624e4b --- /dev/null +++ b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/EscapeAttributesInChoicesPage.html @@ -0,0 +1,20 @@ +<!DOCTYPE html> +<html xmlns:wicket="http://wicket.apache.org";> + <head> + <meta charset="utf-8" /> + <title>Apache Wicket Quickstart</title> + </head> + <body> + <div id="bd"> + <p>Radio group example:</p> + <p> + <span wicket:id="radiofield"></span> + </p> + <p>Dropdown example:</p> + <select wicket:id="dropdownfield"/> + + <p>Checkbox example:</p> + <span wicket:id="checkboxfield"></span> + </div> + </body> +</html> http://git-wip-us.apache.org/repos/asf/wicket/blob/e213b0d2/wicket-core/src/test/java/org/apache/wicket/markup/html/form/EscapeAttributesInChoicesPage.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/form/EscapeAttributesInChoicesPage.java b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/EscapeAttributesInChoicesPage.java new file mode 100644 index 0000000..a6051fa --- /dev/null +++ b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/EscapeAttributesInChoicesPage.java @@ -0,0 +1,53 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.wicket.markup.html.form; + +import org.apache.wicket.markup.html.WebPage; +import org.apache.wicket.request.mapper.parameter.PageParameters; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Map; + +public class EscapeAttributesInChoicesPage extends WebPage { + private static final long serialVersionUID = 1L; + + public EscapeAttributesInChoicesPage(final PageParameters parameters) { + super(parameters); + + final Map<String, String> fruits = new HashMap<>(); + fruits.put("apple\" onmouseover=\"alert('hi');\" \"", "Apple"); + + IChoiceRenderer<String> iChoiceRenderer = new ChoiceRenderer<String>() { + @Override + public Object getDisplayValue(final String s) { + return fruits.get(s); + } + + @Override + public String getIdValue(final String s, final int i) { + return s; + } + }; + + add(new RadioChoice<>("radiofield", new ArrayList<>(fruits.keySet()), iChoiceRenderer)); + add(new DropDownChoice<>("dropdownfield", new ArrayList<>(fruits.keySet()), iChoiceRenderer)); + add(new CheckBoxMultipleChoice<>("checkboxfield", new ArrayList<>(fruits.keySet()), iChoiceRenderer)); + + + } +} http://git-wip-us.apache.org/repos/asf/wicket/blob/e213b0d2/wicket-core/src/test/java/org/apache/wicket/markup/html/form/EscapeAttributesInChoicesTest.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/form/EscapeAttributesInChoicesTest.java b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/EscapeAttributesInChoicesTest.java new file mode 100644 index 0000000..0953587 --- /dev/null +++ b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/EscapeAttributesInChoicesTest.java @@ -0,0 +1,53 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.wicket.markup.html.form; + +import org.apache.wicket.util.tester.TagTester; +import org.apache.wicket.util.tester.WicketTestCase; +import org.junit.Test; + +import static org.apache.wicket.util.tester.TagTester.createTagByAttribute; + +/** + * Test XSS vulnerability in RadioChoice and CheckBoxMultipleChoice + */ +public class EscapeAttributesInChoicesTest extends WicketTestCase +{ + @Test + public void escapeAttributes() + { + tester.startPage(EscapeAttributesInChoicesPage.class); + String lastResponseAsString = tester.getLastResponseAsString(); + + TagTester radioTagTester = tester.getTagById("radiofield1-apple\" onmouseover=\"alert('hi');\" \""); + assertNotNull(radioTagTester); + assertNull(radioTagTester.getAttribute("onmouseover")); + + TagTester dropDownChoiceOptionTagTester = createTagByAttribute(lastResponseAsString, "value", "apple\" onmouseover=\"alert('hi');\" \""); + assertNotNull(dropDownChoiceOptionTagTester); + assertNull(dropDownChoiceOptionTagTester.getAttribute("onmouseover")); + + TagTester checkBoxMultipleChoiceTagTester = createTagByAttribute(lastResponseAsString, "name", "checkboxfield"); + assertNotNull(checkBoxMultipleChoiceTagTester); + assertEquals("apple\" onmouseover=\"alert('hi');\" \"", checkBoxMultipleChoiceTagTester.getAttribute("value")); + assertNull(checkBoxMultipleChoiceTagTester.getAttribute("onmouseover")); + + TagTester labelForCheckBoxMultipleChoiceTagTester = createTagByAttribute(lastResponseAsString, "for", "checkboxfield2-checkboxfield_apple\" onmouseover=\"alert('hi');\" \""); + assertNotNull(labelForCheckBoxMultipleChoiceTagTester); + assertNull(labelForCheckBoxMultipleChoiceTagTester.getAttribute("onmouseover")); + } +}
