You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by sc...@apache.org on 2022/08/08 16:57:52 UTC
[tomcat] 01/03: Propertly-escape role and group information when writing MemoryUserDatabase to an XML file.
This is an automated email from the ASF dual-hosted git repository.
schultz pushed a commit to branch 10.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit cb5052b28443680e2d1b144723cd306f5931df00
Author: Christopher Schultz <ch...@christopherschultz.net>
AuthorDate: Wed Aug 3 13:18:51 2022 -0400
Propertly-escape role and group information when writing MemoryUserDatabase to an XML file.
---
java/org/apache/catalina/users/MemoryGroup.java | 10 +++---
java/org/apache/catalina/users/MemoryRole.java | 6 ++--
.../catalina/users/MemoryUserDatabaseTests.java | 40 ++++++++++++++++++++++
webapps/docs/changelog.xml | 4 +++
4 files changed, 53 insertions(+), 7 deletions(-)
diff --git a/java/org/apache/catalina/users/MemoryGroup.java b/java/org/apache/catalina/users/MemoryGroup.java
index f1008ff80c..dfd02c4dcf 100644
--- a/java/org/apache/catalina/users/MemoryGroup.java
+++ b/java/org/apache/catalina/users/MemoryGroup.java
@@ -20,7 +20,7 @@ package org.apache.catalina.users;
import org.apache.catalina.Role;
import org.apache.catalina.UserDatabase;
import org.apache.tomcat.util.buf.StringUtils;
-
+import org.apache.tomcat.util.security.Escape;
/**
* <p>Concrete implementation of {@link org.apache.catalina.Group} for the
@@ -52,15 +52,17 @@ public class MemoryGroup extends GenericGroup<MemoryUserDatabase> {
@Override
public String toString() {
StringBuilder sb = new StringBuilder("<group groupname=\"");
- sb.append(groupname);
+ sb.append(Escape.xml(groupname));
sb.append("\"");
if (description != null) {
sb.append(" description=\"");
- sb.append(description);
+ sb.append(Escape.xml(description));
sb.append("\"");
}
sb.append(" roles=\"");
- StringUtils.join(roles, ',', Role::getRolename, sb);
+ StringBuilder rsb = new StringBuilder();
+ StringUtils.join(roles, ',', (x) -> Escape.xml(x.getRolename()), rsb);
+ sb.append(rsb);
sb.append("\"");
sb.append("/>");
return sb.toString();
diff --git a/java/org/apache/catalina/users/MemoryRole.java b/java/org/apache/catalina/users/MemoryRole.java
index 10f6d22548..3f0f5855c7 100644
--- a/java/org/apache/catalina/users/MemoryRole.java
+++ b/java/org/apache/catalina/users/MemoryRole.java
@@ -18,7 +18,7 @@ package org.apache.catalina.users;
import org.apache.catalina.UserDatabase;
-
+import org.apache.tomcat.util.security.Escape;
/**
* <p>Concrete implementation of {@link org.apache.catalina.Role} for the
@@ -50,11 +50,11 @@ public class MemoryRole extends GenericRole<MemoryUserDatabase> {
@Override
public String toString() {
StringBuilder sb = new StringBuilder("<role rolename=\"");
- sb.append(rolename);
+ sb.append(Escape.xml(rolename));
sb.append("\"");
if (description != null) {
sb.append(" description=\"");
- sb.append(description);
+ sb.append(Escape.xml(description));
sb.append("\"");
}
sb.append("/>");
diff --git a/test/org/apache/catalina/users/MemoryUserDatabaseTests.java b/test/org/apache/catalina/users/MemoryUserDatabaseTests.java
index 5724ac7829..fa97f93e6b 100644
--- a/test/org/apache/catalina/users/MemoryUserDatabaseTests.java
+++ b/test/org/apache/catalina/users/MemoryUserDatabaseTests.java
@@ -33,6 +33,8 @@ import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;
+import org.apache.catalina.Group;
+import org.apache.catalina.Role;
import org.apache.catalina.User;
import org.apache.catalina.realm.GenericPrincipal;
import org.apache.catalina.realm.UserDatabaseRealm;
@@ -216,4 +218,42 @@ public class MemoryUserDatabaseTests {
Assert.assertEquals(expectedNames.length, j);
}
+
+ @Test
+ public void testDataEscaping() throws Exception {
+ File file = File.createTempFile("tomcat-users", ".xml");
+ file.deleteOnExit();
+
+ MemoryUserDatabase mud = new MemoryUserDatabase();
+ Role role = mud.createRole("role\"name", "descr&iption");
+ Group group = mud.createGroup("grou<p", null);
+ group.addRole(role);
+ Role role2 = mud.createRole("role2", null);
+ group.addRole(role2);
+ User user = mud.createUser("xml<breaker", "\"bobby", "tab'les");
+ user.addRole(role);
+ user.addRole(role2);
+ user.addGroup(group);
+ mud.setPathname(file.getAbsolutePath());
+ mud.setReadonly(false);
+ mud.save();
+
+ String xml;
+ try(java.io.FileReader in = new java.io.FileReader(file)) {
+ StringBuilder sb = new StringBuilder((int)file.length());
+ char[] buffer = new char[4096];
+ int c;
+ while(-1 != (c = in.read(buffer))) {
+ sb.append(buffer, 0, c);
+ }
+ xml = sb.toString();
+ }
+
+ Assert.assertTrue("Role is not properly-escaped",
+ xml.contains("<role rolename=\"role"name\" description=\"descr&iption\""));
+ Assert.assertTrue("Group is not escaped properly",
+ xml.contains("<group groupname=\"grou<p\" roles=\"role"name,role2\""));
+ Assert.assertTrue("User is not properly-escaped",
+ xml.contains("<user username=\"xml<breaker\" password=\""bobby\" fullName=\"tab'les\" groups=\"grou<p\" roles=\"role"name,role2\""));
+ }
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 53035a335c..88b3cea52f 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -119,6 +119,10 @@
Implement the clarification in RFC 9110 that the units in HTTP range
specifiers are case insensitive. (markt)
</fix>
+ <fix>
+ Propertly-escape role and group information when writing
+ MemoryUserDatabase to an XML file. (schultz)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org