CVE-2023-0300: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: Cross-site Scripting (XSS) - Reflected in GitHub repository alfio-event/alf.io prior to 2.0-M4-2301.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 5.4
Published: 2023-01-14T18:15Z
Repository: https://github.com/alfio-event/alf.io
Language: Java
Files changed: src/main/java/alfio/manager/GroupManager.java, src/main/java/alfio/repository/GroupRepository.java, src/main/webapp/resources/js/admin/service/service.js, src/test/java/alfio/manager/GroupManagerIntegrationTest.java
Commit date: 2022-12-09 18:22:07 +0100
Commit message: escape HTML in group name and description (#1155)


Diff: diff --git a/src/main/java/alfio/manager/GroupManager.java b/src/main/java/alfio/manager/GroupManager.java
index 25e41dca..04758c17 100644
--- a/src/main/java/alfio/manager/GroupManager.java
+++ b/src/main/java/alfio/manager/GroupManager.java
@@ -33,10 +33,11 @@ import ch.digitalfondue.npjt.AffectedRowCountAndKey;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.log4j.Log4j2;
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.Validate;
+import org.apache.commons.text.StringEscapeUtils;
 import org.springframework.stereotype.Component;
 import org.springframework.transaction.PlatformTransactionManager;
 import org.springframework.transaction.TransactionDefinition;
 import org.springframework.transaction.annotation.Transactional;
 import org.springframework.transaction.support.DefaultTransactionDefinition;
@@ -47,10 +48,11 @@ import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
 import static alfio.model.group.LinkedGroup.MatchType.FULL;
 import static alfio.model.group.LinkedGroup.Type.*;
 import static java.util.Collections.singletonList;
+import static org.apache.commons.text.StringEscapeUtils.escapeHtml4;
 
 @Component
 @Log4j2
 public class GroupManager {
 
@@ -79,11 +81,11 @@ public class GroupManager {
             return insertMembers;
         });
     }
 
     Group createNew(String name, String description, int organizationId) {
-        AffectedRowCountAndKey<Integer> insert = groupRepository.insert(name, description, organizationId);
+        AffectedRowCountAndKey<Integer> insert = groupRepository.insert(escapeHtml4(name), escapeHtml4(description), organizationId);
         return groupRepository.getById(insert.getKey());
     }
 
     @Transactional
     public LinkedGroup createLink(int groupId,
@@ -245,11 +247,11 @@ public class GroupManager {
             if(!insertResult.isSuccess()) {
                 var error = Objects.requireNonNull(insertResult.getFirstErrorOrNull());
                 throw new DuplicateGroupItemException(error.getDescription());
             }
         }
-        groupRepository.update(listId, modification.getName(), modification.getDescription());
+        groupRepository.update(listId, escapeHtml4(modification.getName()), escapeHtml4(modification.getDescription()));
         return loadComplete(listId);
     }
 
     @Transactional
     public boolean deactivateMembers(List<Integer> memberIds, int groupId) {
diff --git a/src/main/java/alfio/repository/GroupRepository.java b/src/main/java/alfio/repository/GroupRepository.java
index 1e1bb4f5..2f64f39f 100644
--- a/src/main/java/alfio/repository/GroupRepository.java
+++ b/src/main/java/alfio/repository/GroupRepository.java
@@ -26,10 +26,12 @@ import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
 
 import java.util.List;
 import java.util.Optional;
 import java.util.UUID;
 
+import static org.apache.commons.text.StringEscapeUtils.escapeHtml4;
+
 @QueryRepository
 public interface GroupRepository {
 
     String BY_EVENT_ID = "select * from group_link_active where event_id_fk = :eventId";
 
@@ -64,11 +66,13 @@ public interface GroupRepository {
                                                         @Bind("matchType") LinkedGroup.MatchType matchType,
                                                         @Bind("maxAllocation") Integer maxAllocation);
 
     default int[] insert(int groupId, List<GroupMemberModification> members) {
         MapSqlParameterSource[] params = members.stream()
-            .map(i -> new MapSqlParameterSource("groupId", groupId).addValue("value", i.getValue().toLowerCase()).addValue("description", i.getDescription()))
+            .map(i -> new MapSqlParameterSource("groupId", groupId)
+                .addValue("value", i.getValue().toLowerCase())
+                .addValue("description", escapeHtml4(i.getDescription())))
             .toArray(MapSqlParameterSource[]::new);
 
         return getNamedParameterJdbcTemplate().batchUpdate("insert into group_member(a_group_id_fk, value, description) values(:groupId, :value, :description)", params);
     }
 
diff --git a/src/main/webapp/resources/js/admin/service/service.js b/src/main/webapp/resources/js/admin/service/service.js
index 96b0b316..2f19d99c 100644
--- a/src/main/webapp/resources/js/admin/service/service.js
+++ b/src/main/webapp/resources/js/admin/service/service.js
@@ -620,24 +620,30 @@
                 NotificationHandler.showError(message);
             }
         };
     }]);
 
-    baseServices.service("NotificationHandler", ["growl", function (growl) {
+    baseServices.service("NotificationHandler", ["growl", "$sanitize", function (growl, $sanitize) {
         var config = {ttl: 5000, disableCountDown: true};
+        var sanitize = function(message) {
+            var sanitized = $sanitize(message);
+            return sanitized.split(' ').map(function(part) {
+                return encodeURIComponent(part);
+            }).join(' ');
+        };
         return {
             showSuccess: function (message) {
-                return growl.success(message, config);
+                return growl.success(sanitize(message), config);
             },
             showWarning: function (message) {
-                return growl.warning(message, config);
+                return growl.warning(sanitize(message), config);
             },
             showInfo : function (message) {
-                return growl.info(message, config);
+                return growl.info(sanitize(message), config);
             },
             showError : function (message) {
-                return growl.error(message, config);
+                return growl.error(sanitize(message), config);
             }
         }
 
     }]);
 
diff --git a/src/test/java/alfio/manager/GroupManagerIntegrationTest.java b/src/test/java/alfio/manager/GroupManagerIntegrationTest.java
index b53ecc60..1ae8de07 100644
--- a/src/test/java/alfio/manager/GroupManagerIntegrationTest.java
+++ b/src/test/java/alfio/manager/GroupManagerIntegrationTest.java
@@ -158,6 +158,32 @@ public class GroupManagerIntegrationTest extends BaseIntegrationTest {
         Result<Integer> items = groupManager.insertMembers(group.getId(), Arrays.asList(new GroupMemberModification(null,"test@test.ch", "description"), new GroupMemberModification(null,"test@test.ch", "description")));
         Assertions.assertFalse(items.isSuccess());
         assertEquals("value.duplicate", items.getFirstErrorOrNull().getCode());
         assertEquals("test@test.ch", items.getFirstErrorOrNull().getDescription());
     }
+
+    @Test
+    void testEscape() {
+        List<TicketCategoryModification> categories = Collections.singletonList(
+            new TicketCategoryModification(null, "default", TicketCategory.TicketAccessType.INHERIT, 10,
+                new DateTimeModification(LocalDate.now(ClockProvider.clock()).plusDays(1), LocalTime.now(ClockProvider.clock())),
+                new DateTimeModification(LocalDate.now(ClockProvider.clock()).plusDays(2), LocalTime.now(ClockProvider.clock())),
+                DESCRIPTION, BigDecimal.TEN, false, "", false, null, null, null, null, null, 0, null, null, AlfioMetadata.empty()));
+        Pair<Event, String> pair = initEvent(categories, organizationRepository, userManager, eventManager, eventRepository);
+        Event event = pair.getKey();
+        Group group = groupManager.createNew("test > 1", "This is a test < 1", event.getOrganizationId());
+        assertNotNull(group);
+        assertEquals("This is a test &lt; 1", group.getDescription());
+        assertEquals("test &gt; 1", group.getName());
+        LinkedGroupModification modification = new LinkedGroupModification(null, group.getId(), event.getId(), null, LinkedGroup.Type.ONCE_PER_VALUE, LinkedGroup.MatchType.FULL, null);
+        LinkedGroup configuration = groupManager.createLink(group.getId(), event.getId(), modification);
+        assertNotNull(configuration);
+        Result<Integer> items = groupManager.insertMembers(group.getId(), List.of(new GroupMemberModification(null,"test@test.ch", "description <>")));
+        assertTrue(items.isSuccess());
+        var persistedGroup = groupManager.loadComplete(group.getId()).orElseThrow();
+        assertEquals("description &lt;&gt;", persistedGroup.getItems().get(0).getDescription());
+        groupManager.update(group.getId(), new GroupModification(group.getId(), "test > 1", "This is a test < 1", event.getOrganizationId(), List.of(new GroupMemberModification(null,"test@test.ch", "description <>"))));
+        persistedGroup = groupManager.loadComplete(group.getId()).orElseThrow();
+        assertEquals("This is a test &lt; 1", persistedGroup.getDescription());
+        assertEquals("test &gt; 1", persistedGroup.getName());
+    }
 }
\ No newline at end of file

Vulnerable code:         AffectedRowCountAndKey<Integer> insert = groupRepository.insert(name, description, organizationId);
        groupRepository.update(listId, modification.getName(), modification.getDescription());
            .map(i -> new MapSqlParameterSource("groupId", groupId).addValue("value", i.getValue().toLowerCase()).addValue("description", i.getDescription()))
    baseServices.service("NotificationHandler", ["growl", function (growl) {
                return growl.success(message, config);
                return growl.warning(message, config);
                return growl.info(message, config);
                return growl.error(message, config);
Fixed code: import org.apache.commons.text.StringEscapeUtils;
import static org.apache.commons.text.StringEscapeUtils.escapeHtml4;
        AffectedRowCountAndKey<Integer> insert = groupRepository.insert(escapeHtml4(name), escapeHtml4(description), organizationId);
        groupRepository.update(listId, escapeHtml4(modification.getName()), escapeHtml4(modification.getDescription()));
import static org.apache.commons.text.StringEscapeUtils.escapeHtml4;

            .map(i -> new MapSqlParameterSource("groupId", groupId)
                .addValue("value", i.getValue().toLowerCase())
                .addValue("description", escapeHtml4(i.getDescription())))
    baseServices.service("NotificationHandler", ["growl", "$sanitize", function (growl, $sanitize) {
        var sanitize = function(message) {
            var sanitized = $sanitize(message);
            return sanitized.split(' ').map(function(part) {
                return encodeURIComponent(part);
            }).join(' ');
        };
                return growl.success(sanitize(message), config);
                return growl.warning(sanitize(message), config);
                return growl.info(sanitize(message), config);
                return growl.error(sanitize(message), config);

    @Test
    void testEscape() {
        List<TicketCategoryModification> categories = Collections.singletonList(
            new TicketCategoryModification(null, "default", TicketCategory.TicketAccessType.INHERIT, 10,
                new DateTimeModification(LocalDate.now(ClockProvider.clock()).plusDays(1), LocalTime.now(ClockProvider.clock())),
                new DateTimeModification(LocalDate.now(ClockProvider.clock()).plusDays(2), LocalTime.now(ClockProvider.clock())),
                DESCRIPTION, BigDecimal.TEN, false, "", false, null, null, null, null, null, 0, null, null, AlfioMetadata.empty()));
        Pair<Event, String> pair = initEvent(categories, organizationRepository, userManager, eventManager, eventRepository);
        Event event = pair.getKey();
        Group group = groupManager.createNew("test > 1", "This is a test < 1", event.getOrganizationId());
        assertNotNull(group);
        assertEquals("This is a test &lt; 1", group.getDescription());
        assertEquals("test &gt; 1", group.getName());
        LinkedGroupModification modification = new LinkedGroupModification(null, group.getId(), event.getId(), null, LinkedGroup.Type.ONCE_PER_VALUE, LinkedGroup.MatchType.FULL, null);
        LinkedGroup configuration = groupManager.createLink(group.getId(), event.getId(), modification);
        assertNotNull(configuration);
        Result<Integer> items = groupManager.insertMembers(group.getId(), List.of(new GroupMemberModification(null,"test@test.ch", "description <>")));
        assertTrue(items.isSuccess());
        var persistedGroup = groupManager.loadComplete(group.getId()).orElseThrow();
        assertEquals("description &lt;&gt;", persistedGroup.getItems().get(0).getDescription());
        groupManager.update(group.getId(), new GroupModification(group.getId(), "test > 1", "This is a test < 1", event.getOrganizationId(), List.of(new GroupMemberModification(null,"test@test.ch", "description <>"))));
        persistedGroup = groupManager.loadComplete(group.getId()).orElseThrow();
        assertEquals("This is a test &lt; 1", persistedGroup.getDescription());
        assertEquals("test &gt; 1", persistedGroup.getName());
    }