RHPAM-4719: Persistent Cross-Site Scripting (XSS)

Fixes RHPAM-4716 & RHPAM-4717 by using StringEscapeUtils:: escapeHtml4() method in ProjectResource and by implementing helper method, using escapeHtml4(), to escape conrtributors names in OrganizationalUnitServiceImpl
kiegroup · Jun 15, 2023 · 4272615 · 4272615
1 parent a8a7590
commit 4272615
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 5 deletions.
diff --git a/uberfire-rest/uberfire-rest-backend/pom.xml b/uberfire-rest/uberfire-rest-backend/pom.xml
@@ -72,6 +72,11 @@
       <artifactId>slf4j-api</artifactId>
     </dependency>
 
+    <dependency>
+      <groupId>org.apache.commons</groupId>
+      <artifactId>commons-text</artifactId>
+    </dependency>
+
     <!-- REST dependencies -->
     <dependency>
       <groupId>jakarta.annotation</groupId>

diff --git a/...ire-rest/uberfire-rest-backend/src/main/java/org/guvnor/rest/backend/ProjectResource.java b/...ire-rest/uberfire-rest-backend/src/main/java/org/guvnor/rest/backend/ProjectResource.java
@@ -46,6 +46,7 @@
 import javax.ws.rs.core.UriInfo;
 import javax.ws.rs.core.Variant;
 
+import org.apache.commons.text.StringEscapeUtils;
 import org.guvnor.common.services.project.model.WorkspaceProject;
 import org.guvnor.common.services.project.service.WorkspaceProjectService;
 import org.guvnor.rest.client.AddBranchJobRequest;
@@ -373,7 +374,7 @@ public Response addBranch(@PathParam("spaceName") String spaceName,
         jobRequest.setJobId(id);
         jobRequest.setSpaceName(spaceName);
         jobRequest.setProjectName(projectName);
-        jobRequest.setNewBranchName(addBranchRequest.getNewBranchName());
+        jobRequest.setNewBranchName(StringEscapeUtils.escapeHtml4(addBranchRequest.getNewBranchName()));
         jobRequest.setBaseBranchName(addBranchRequest.getBaseBranchName());
         jobRequest.setUserIdentifier(sessionInfo.getIdentity().getIdentifier());
         addAcceptedJobResult(id);
@@ -684,7 +685,7 @@ public Response createSpace(Space space) {
         jobRequest.setJobId(id);
         jobRequest.setSpaceName(space.getName());
         jobRequest.setDescription(space.getDescription());
-        jobRequest.setOwner(space.getOwner());
+        jobRequest.setOwner(StringEscapeUtils.escapeHtml4(space.getOwner()));
         jobRequest.setDefaultGroupId(space.getDefaultGroupId());
         addAcceptedJobResult(id);
 
@@ -709,7 +710,7 @@ public Response updateSpace(Space space) {
         jobRequest.setJobId(id);
         jobRequest.setSpaceName(space.getName());
         jobRequest.setDescription(space.getDescription());
-        jobRequest.setOwner(space.getOwner());
+        jobRequest.setOwner(StringEscapeUtils.escapeHtml4(space.getOwner()));
         jobRequest.setDefaultGroupId(space.getDefaultGroupId());
         addAcceptedJobResult(id);
 

diff --git a/uberfire-structure/uberfire-structure-backend/pom.xml b/uberfire-structure/uberfire-structure-backend/pom.xml
@@ -140,6 +140,11 @@
       <artifactId>commons-fileupload</artifactId>
     </dependency>
 
+    <dependency>
+      <groupId>org.apache.commons</groupId>
+      <artifactId>commons-text</artifactId>
+    </dependency>
+
     <dependency>
       <groupId>org.uberfire</groupId>
       <artifactId>uberfire-security-management-api</artifactId>

diff --git a/...n/java/org/guvnor/structure/backend/organizationalunit/OrganizationalUnitServiceImpl.java b/...n/java/org/guvnor/structure/backend/organizationalunit/OrganizationalUnitServiceImpl.java
@@ -33,6 +33,7 @@
 import javax.inject.Inject;
 import javax.inject.Named;
 
+import org.apache.commons.text.StringEscapeUtils;
 import org.uberfire.security.Contributor;
 import org.guvnor.structure.contributors.SpaceContributorsUpdatedEvent;
 import org.guvnor.structure.organizationalunit.NewOrganizationalUnitEvent;
@@ -320,7 +321,7 @@ public OrganizationalUnit createOrganizationalUnit(final String name,
             final SpaceInfo spaceInfo = new SpaceInfo(name,
                                                       description,
                                                       _defaultGroupId,
-                                                      contributors,
+                                                      escapeContributorsNames(contributors),
                                                       getRepositoryAliases(repositories),
                                                       Collections.emptyList());
             spaceConfigStorageRegistry.get(name).saveSpaceInfo(spaceInfo);
@@ -378,7 +379,7 @@ public OrganizationalUnit updateOrganizationalUnit(String name,
                         spaceInfo.setDefaultGroupId(_defaultGroupId);
 
                         if (contributors != null) {
-                            spaceInfo.setContributors(contributors);
+                            spaceInfo.setContributors(escapeContributorsNames(contributors));
                         }
 
                         if (description != null) {
@@ -619,4 +620,13 @@ private OrganizationalUnit createDeletedOrganizationalUnit(ConfigGroup configGro
                                           defaultGroupId,
                                           true);
     }
+
+    private Collection<Contributor> escapeContributorsNames(Collection<Contributor> contributors) {
+        Collection<Contributor> escapedContributors = new ArrayList<>();
+        contributors.forEach((contributor -> {
+            String escapedName = StringEscapeUtils.escapeHtml4(contributor.getUsername());
+            escapedContributors.add(new Contributor(escapedName, contributor.getType()));
+        }));
+        return escapedContributors;
+    }
 }