From 6356bb9f8e1802fb62f76f2f7e79b617168067ff Mon Sep 17 00:00:00 2001 From: Igor Dianov Date: Wed, 11 Dec 2024 17:11:09 -0800 Subject: [PATCH] APPSEC-2909 Fix SQL injection vulnerability in Activiti Cloud Query Search Api --- .../BigDecimalVariableValueCondition.java | 5 +- .../DateVariableValueCondition.java | 5 +- .../DatetimeVariableValueCondition.java | 6 +- .../StringVariableValueCondition.java | 2 +- .../specification/SpecificationSupportIT.java | 135 ++++++++++++++++++ 5 files changed, 146 insertions(+), 7 deletions(-) diff --git a/activiti-cloud-query-service/activiti-cloud-services-query/activiti-cloud-services-query-rest/src/main/java/org/activiti/cloud/services/query/rest/specification/BigDecimalVariableValueCondition.java b/activiti-cloud-query-service/activiti-cloud-services-query/activiti-cloud-services-query-rest/src/main/java/org/activiti/cloud/services/query/rest/specification/BigDecimalVariableValueCondition.java index 922bde82a7..6c82945fb5 100644 --- a/activiti-cloud-query-service/activiti-cloud-services-query/activiti-cloud-services-query-rest/src/main/java/org/activiti/cloud/services/query/rest/specification/BigDecimalVariableValueCondition.java +++ b/activiti-cloud-query-service/activiti-cloud-services-query/activiti-cloud-services-query-rest/src/main/java/org/activiti/cloud/services/query/rest/specification/BigDecimalVariableValueCondition.java @@ -17,6 +17,7 @@ import jakarta.persistence.criteria.CriteriaBuilder; import jakarta.persistence.criteria.Path; +import java.math.BigDecimal; import org.activiti.cloud.services.query.rest.filter.FilterOperator; import org.activiti.cloud.services.query.rest.filter.VariableType; @@ -37,7 +38,7 @@ public BigDecimalVariableValueCondition( } @Override - protected Object getConvertedValue() { - return value; + protected BigDecimal getConvertedValue() { + return BigDecimal.valueOf(Double.parseDouble(value)); } } diff --git a/activiti-cloud-query-service/activiti-cloud-services-query/activiti-cloud-services-query-rest/src/main/java/org/activiti/cloud/services/query/rest/specification/DateVariableValueCondition.java b/activiti-cloud-query-service/activiti-cloud-services-query/activiti-cloud-services-query-rest/src/main/java/org/activiti/cloud/services/query/rest/specification/DateVariableValueCondition.java index 57d94b5b96..e071aadee4 100644 --- a/activiti-cloud-query-service/activiti-cloud-services-query/activiti-cloud-services-query-rest/src/main/java/org/activiti/cloud/services/query/rest/specification/DateVariableValueCondition.java +++ b/activiti-cloud-query-service/activiti-cloud-services-query/activiti-cloud-services-query-rest/src/main/java/org/activiti/cloud/services/query/rest/specification/DateVariableValueCondition.java @@ -17,6 +17,7 @@ import jakarta.persistence.criteria.CriteriaBuilder; import jakarta.persistence.criteria.Path; +import java.time.LocalDate; import org.activiti.cloud.dialect.CustomPostgreSQLDialect; import org.activiti.cloud.services.query.rest.exception.IllegalFilterException; import org.activiti.cloud.services.query.rest.filter.FilterOperator; @@ -47,7 +48,7 @@ protected String getFunctionName() { } @Override - protected String getConvertedValue() { - return value; + protected LocalDate getConvertedValue() { + return LocalDate.parse(value); } } diff --git a/activiti-cloud-query-service/activiti-cloud-services-query/activiti-cloud-services-query-rest/src/main/java/org/activiti/cloud/services/query/rest/specification/DatetimeVariableValueCondition.java b/activiti-cloud-query-service/activiti-cloud-services-query/activiti-cloud-services-query-rest/src/main/java/org/activiti/cloud/services/query/rest/specification/DatetimeVariableValueCondition.java index 02045237b3..44c2deff94 100644 --- a/activiti-cloud-query-service/activiti-cloud-services-query/activiti-cloud-services-query-rest/src/main/java/org/activiti/cloud/services/query/rest/specification/DatetimeVariableValueCondition.java +++ b/activiti-cloud-query-service/activiti-cloud-services-query/activiti-cloud-services-query-rest/src/main/java/org/activiti/cloud/services/query/rest/specification/DatetimeVariableValueCondition.java @@ -17,6 +17,8 @@ import jakarta.persistence.criteria.CriteriaBuilder; import jakarta.persistence.criteria.Path; +import java.time.Instant; +import java.util.Date; import org.activiti.cloud.dialect.CustomPostgreSQLDialect; import org.activiti.cloud.services.query.rest.exception.IllegalFilterException; import org.activiti.cloud.services.query.rest.filter.FilterOperator; @@ -47,7 +49,7 @@ protected String getFunctionName() { } @Override - protected String getConvertedValue() { - return value; + protected Date getConvertedValue() { + return Date.from(Instant.parse(value)); } } diff --git a/activiti-cloud-query-service/activiti-cloud-services-query/activiti-cloud-services-query-rest/src/main/java/org/activiti/cloud/services/query/rest/specification/StringVariableValueCondition.java b/activiti-cloud-query-service/activiti-cloud-services-query/activiti-cloud-services-query-rest/src/main/java/org/activiti/cloud/services/query/rest/specification/StringVariableValueCondition.java index f2896e4a95..672989c6c2 100644 --- a/activiti-cloud-query-service/activiti-cloud-services-query/activiti-cloud-services-query-rest/src/main/java/org/activiti/cloud/services/query/rest/specification/StringVariableValueCondition.java +++ b/activiti-cloud-query-service/activiti-cloud-services-query/activiti-cloud-services-query-rest/src/main/java/org/activiti/cloud/services/query/rest/specification/StringVariableValueCondition.java @@ -45,6 +45,6 @@ protected String getFunctionName() { @Override protected String getConvertedValue() { - return value; + return value.replaceAll("'", "''").replaceAll("\"", ""); } } diff --git a/activiti-cloud-query-service/activiti-cloud-services-query/activiti-cloud-services-query-rest/src/test/java/org/activiti/cloud/services/query/rest/specification/SpecificationSupportIT.java b/activiti-cloud-query-service/activiti-cloud-services-query/activiti-cloud-services-query-rest/src/test/java/org/activiti/cloud/services/query/rest/specification/SpecificationSupportIT.java index afd4c632b8..b11146e1be 100644 --- a/activiti-cloud-query-service/activiti-cloud-services-query/activiti-cloud-services-query-rest/src/test/java/org/activiti/cloud/services/query/rest/specification/SpecificationSupportIT.java +++ b/activiti-cloud-query-service/activiti-cloud-services-query/activiti-cloud-services-query-rest/src/test/java/org/activiti/cloud/services/query/rest/specification/SpecificationSupportIT.java @@ -18,10 +18,15 @@ import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy; import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat; +import jakarta.persistence.EntityManager; import jakarta.persistence.criteria.CriteriaBuilder; import jakarta.persistence.criteria.CriteriaQuery; import jakarta.persistence.criteria.Predicate; import jakarta.persistence.criteria.Root; +import java.math.BigDecimal; +import java.time.Instant; +import java.time.LocalDate; +import java.util.Date; import java.util.List; import java.util.stream.Stream; import org.activiti.cloud.services.query.app.repository.VariableRepository; @@ -31,6 +36,7 @@ import org.activiti.cloud.services.query.rest.filter.VariableFilter; import org.activiti.cloud.services.query.rest.filter.VariableType; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -54,6 +60,9 @@ class SpecificationSupportIT { @Autowired VariableRepository variableRepository; + @Autowired + EntityManager entityManager; + @Container @ServiceConnection static PostgreSQLContainer postgres = new PostgreSQLContainer<>("postgres:15-alpine"); @@ -192,6 +201,132 @@ void should_findEntitiesByVariableValueUsingSpecification( assertThat(retrieved).containsExactlyInAnyOrderElementsOf(variables.subList(0, expectedSublistToIndex)); } + @Test + public void testStringVariableValueConditionConvertedValue() { + var criteriaBuilder = entityManager.getCriteriaBuilder(); + var query = criteriaBuilder.createQuery(ProcessVariableEntity.class); + var root = query.from(ProcessVariableEntity.class); + + // given + var subject = new StringVariableValueCondition( + root.get(ProcessVariableEntity_.value), + FilterOperator.EQUALS, + "va'lue\"", + criteriaBuilder + ); + + // when + var value = subject.getConvertedValue(); + + // then + assertThat(value).isEqualTo("va''lue"); + } + + @Test + public void testBooleanVariableValueConditionConvertedValue() { + var criteriaBuilder = entityManager.getCriteriaBuilder(); + var query = criteriaBuilder.createQuery(ProcessVariableEntity.class); + var root = query.from(ProcessVariableEntity.class); + + // given + var subject = new BooleanVariableValueCondition( + root.get(ProcessVariableEntity_.value), + FilterOperator.EQUALS, + "true", + criteriaBuilder + ); + + // when + var value = subject.getConvertedValue(); + + // then + assertThat(value).isEqualTo(Boolean.TRUE); + } + + @Test + public void testBigDecimalVariableValueConditionConvertedValue() { + var criteriaBuilder = entityManager.getCriteriaBuilder(); + var query = criteriaBuilder.createQuery(ProcessVariableEntity.class); + var root = query.from(ProcessVariableEntity.class); + + // given + var subject = new BigDecimalVariableValueCondition( + root.get(ProcessVariableEntity_.value), + FilterOperator.EQUALS, + "10.00", + criteriaBuilder + ); + + // when + var value = subject.getConvertedValue(); + + // then + assertThat(value).isEqualTo(BigDecimal.valueOf(10.00)); + } + + @Test + public void testDatetimeVariableValueConditionConvertedValue() { + var criteriaBuilder = entityManager.getCriteriaBuilder(); + var query = criteriaBuilder.createQuery(ProcessVariableEntity.class); + var root = query.from(ProcessVariableEntity.class); + + // given + var subject = new DatetimeVariableValueCondition( + root.get(ProcessVariableEntity_.value), + FilterOperator.EQUALS, + "2024-08-02T00:11:22.000+00:00", + criteriaBuilder + ); + + // when + var value = subject.getConvertedValue(); + + // then + assertThat(value).isEqualTo(Date.from(Instant.parse("2024-08-02T00:11:22.000+00:00"))); + } + + @Test + public void testDateVariableValueConditionConvertedValue() { + var criteriaBuilder = entityManager.getCriteriaBuilder(); + var query = criteriaBuilder.createQuery(ProcessVariableEntity.class); + var root = query.from(ProcessVariableEntity.class); + + // given + var subject = new DateVariableValueCondition( + root.get(ProcessVariableEntity_.value), + FilterOperator.EQUALS, + "2024-08-02", + criteriaBuilder + ); + + // when + var value = subject.getConvertedValue(); + + // then + assertThat(value).isEqualTo(LocalDate.parse("2024-08-02")); + } + + @Test + public void testIntegerVariableValueConditionConvertedValue() { + var criteriaBuilder = entityManager.getCriteriaBuilder(); + var query = criteriaBuilder.createQuery(ProcessVariableEntity.class); + var root = query.from(ProcessVariableEntity.class); + + // given + var subject = new IntegerVariableValueCondition( + root.get(ProcessVariableEntity_.value), + FilterOperator.EQUALS, + "123", + criteriaBuilder + ); + + // when + var value = subject.getConvertedValue(); + + // then + assertThat(value).isEqualTo(123); + } + @ParameterizedTest @MethodSource("provideArgumentsThatShouldThrow") void should_throw_ResponseStatusException(VariableType variableType, FilterOperator operator) {