You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by mg...@apache.org on 2015/12/21 16:33:14 UTC
wicket git commit: Escape the generated markup for attribute names
and values in CheckBoxMultipleChoice and RadioChoice
Repository: wicket
Updated Branches:
refs/heads/master 274696a92 -> e49e041dc
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/e49e041d
Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/e49e041d
Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/e49e041d
Branch: refs/heads/master
Commit: e49e041dc7d34413365bcddb51d0c50a36d3ed6a
Parents: 274696a
Author: Martin Tzvetanov Grigorov <mg...@apache.org>
Authored: Mon Dec 21 16:31:55 2015 +0100
Committer: Martin Tzvetanov Grigorov <mg...@apache.org>
Committed: Mon Dec 21 16:32:59 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/e49e041d/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 73c5e9e..7823b31 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
@@ -425,7 +425,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:
@@ -451,9 +451,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
@@ -464,9 +464,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('"');
}
}
@@ -498,7 +498,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/e49e041d/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 2ab3b89..a2d3551 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
@@ -477,9 +477,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('"');
}
}
@@ -489,7 +489,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('>')
@@ -518,9 +518,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)
@@ -544,7 +544,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("';\"");
}
}
@@ -557,9 +557,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('"');
}
}
@@ -585,7 +585,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/e49e041d/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/e49e041d/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/e49e041d/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"));
+ }
+}