From 6bc9148f772e724f4ec322d9790665fa3d0c5050 Mon Sep 17 00:00:00 2001 From: dashevchenko Date: Mon, 7 Aug 2023 11:20:53 +0300 Subject: [PATCH] added optional nosxss validation for attribute/telemetry value --- .../DefaultTelemetrySubscriptionService.java | 6 ++++- .../src/main/resources/thingsboard.yml | 2 ++ .../controller/TelemetryControllerTest.java | 17 +++++++++++++ .../server/dao/attributes/AttributeUtils.java | 8 +++--- .../dao/attributes/BaseAttributesService.java | 8 ++++-- .../attributes/CachedAttributesService.java | 6 +++-- .../thingsboard/server/dao/util/KvUtils.java | 25 ++++++++++++------- 7 files changed, 54 insertions(+), 18 deletions(-) diff --git a/application/src/main/java/org/thingsboard/server/service/telemetry/DefaultTelemetrySubscriptionService.java b/application/src/main/java/org/thingsboard/server/service/telemetry/DefaultTelemetrySubscriptionService.java index 3f5e52796a..df3a15e765 100644 --- a/application/src/main/java/org/thingsboard/server/service/telemetry/DefaultTelemetrySubscriptionService.java +++ b/application/src/main/java/org/thingsboard/server/service/telemetry/DefaultTelemetrySubscriptionService.java @@ -21,6 +21,7 @@ import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.MoreExecutors; import com.google.common.util.concurrent.SettableFuture; import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Lazy; import org.springframework.stereotype.Service; import org.thingsboard.common.util.ThingsBoardThreadFactory; @@ -78,6 +79,9 @@ public class DefaultTelemetrySubscriptionService extends AbstractSubscriptionSer private ExecutorService tsCallBackExecutor; + @Value("${sql.ts.value_no_xss_validation:false}") + private boolean valueNoXssValidation; + public DefaultTelemetrySubscriptionService(AttributesService attrService, TimeseriesService tsService, @Lazy TbEntityViewService tbEntityViewService, @@ -135,7 +139,7 @@ public class DefaultTelemetrySubscriptionService extends AbstractSubscriptionSer checkInternalEntity(entityId); boolean sysTenant = TenantId.SYS_TENANT_ID.equals(tenantId) || tenantId == null; if (sysTenant || apiUsageStateService.getApiUsageState(tenantId).isDbStorageEnabled()) { - KvUtils.validate(ts); + KvUtils.validate(ts, valueNoXssValidation); if (saveLatest) { saveAndNotifyInternal(tenantId, entityId, ts, ttl, getCallback(tenantId, customerId, sysTenant, callback)); } else { diff --git a/application/src/main/resources/thingsboard.yml b/application/src/main/resources/thingsboard.yml index aa62a46d61..c8742921b0 100644 --- a/application/src/main/resources/thingsboard.yml +++ b/application/src/main/resources/thingsboard.yml @@ -269,11 +269,13 @@ sql: batch_max_delay: "${SQL_ATTRIBUTES_BATCH_MAX_DELAY_MS:100}" stats_print_interval_ms: "${SQL_ATTRIBUTES_BATCH_STATS_PRINT_MS:10000}" batch_threads: "${SQL_ATTRIBUTES_BATCH_THREADS:3}" # batch thread count have to be a prime number like 3 or 5 to gain perfect hash distribution + value_no_xss_validation: "${SQL_ATTRIBUTES_VALUE_NO_XSS_VALIDATION:false}" ts: batch_size: "${SQL_TS_BATCH_SIZE:10000}" batch_max_delay: "${SQL_TS_BATCH_MAX_DELAY_MS:100}" stats_print_interval_ms: "${SQL_TS_BATCH_STATS_PRINT_MS:10000}" batch_threads: "${SQL_TS_BATCH_THREADS:3}" # batch thread count have to be a prime number like 3 or 5 to gain perfect hash distribution + value_no_xss_validation: "${SQL_TS_VALUE_NO_XSS_VALIDATION:false}" ts_latest: batch_size: "${SQL_TS_LATEST_BATCH_SIZE:10000}" batch_max_delay: "${SQL_TS_LATEST_BATCH_MAX_DELAY_MS:100}" diff --git a/application/src/test/java/org/thingsboard/server/controller/TelemetryControllerTest.java b/application/src/test/java/org/thingsboard/server/controller/TelemetryControllerTest.java index 47cac1b549..fc6fc33b8f 100644 --- a/application/src/test/java/org/thingsboard/server/controller/TelemetryControllerTest.java +++ b/application/src/test/java/org/thingsboard/server/controller/TelemetryControllerTest.java @@ -16,6 +16,7 @@ package org.thingsboard.server.controller; import org.junit.Test; +import org.springframework.test.context.TestPropertySource; import org.thingsboard.server.common.data.Device; import org.thingsboard.server.common.data.SaveDeviceWithCredentialsRequest; import org.thingsboard.server.common.data.security.DeviceCredentials; @@ -25,6 +26,10 @@ import org.thingsboard.server.dao.service.DaoSqlTest; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @DaoSqlTest +@TestPropertySource(properties = { + "sql.attributes.value_no_xss_validation=true", + "sql.ts.value_no_xss_validation=true" +}) public class TelemetryControllerTest extends AbstractControllerTest { @Test @@ -39,6 +44,18 @@ public class TelemetryControllerTest extends AbstractControllerTest { doPostAsync("/api/plugins/telemetry/DEVICE/" + device.getId() + "/timeseries/smth", invalidRequestBody, String.class, status().isBadRequest()); } + @Test + public void testValueConstraintValidator() throws Exception { + loginTenantAdmin(); + Device device = createDevice(); + String correctRequestBody = "{\"data\": \"value\"}"; + doPostAsync("/api/plugins/telemetry/" + device.getId() + "/SHARED_SCOPE", correctRequestBody, String.class, status().isOk()); + doPostAsync("/api/plugins/telemetry/DEVICE/" + device.getId() + "/timeseries/smth", correctRequestBody, String.class, status().isOk()); + String invalidRequestBody = "{\"data\": \"alert(document)\\\">\"}"; + doPostAsync("/api/plugins/telemetry/" + device.getId() + "/SHARED_SCOPE", invalidRequestBody, String.class, status().isBadRequest()); + doPostAsync("/api/plugins/telemetry/DEVICE/" + device.getId() + "/timeseries/smth", invalidRequestBody, String.class, status().isBadRequest()); + } + private Device createDevice() throws Exception { String testToken = "TEST_TOKEN"; diff --git a/dao/src/main/java/org/thingsboard/server/dao/attributes/AttributeUtils.java b/dao/src/main/java/org/thingsboard/server/dao/attributes/AttributeUtils.java index d1abeda5b6..192d56334d 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/attributes/AttributeUtils.java +++ b/dao/src/main/java/org/thingsboard/server/dao/attributes/AttributeUtils.java @@ -30,12 +30,12 @@ public class AttributeUtils { Validator.validateString(scope, "Incorrect scope " + scope); } - public static void validate(List kvEntries) { - kvEntries.forEach(AttributeUtils::validate); + public static void validate(List kvEntries, boolean valueNoXssValidation) { + kvEntries.forEach(tsKvEntry -> validate(tsKvEntry, valueNoXssValidation)); } - public static void validate(AttributeKvEntry kvEntry) { - KvUtils.validate(kvEntry); + public static void validate(AttributeKvEntry kvEntry, boolean valueNoXssValidation) { + KvUtils.validate(kvEntry, valueNoXssValidation); if (kvEntry.getDataType() == null) { throw new IncorrectParameterException("Incorrect kvEntry. Data type can't be null"); } else { diff --git a/dao/src/main/java/org/thingsboard/server/dao/attributes/BaseAttributesService.java b/dao/src/main/java/org/thingsboard/server/dao/attributes/BaseAttributesService.java index 09414ac750..f855c116e2 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/attributes/BaseAttributesService.java +++ b/dao/src/main/java/org/thingsboard/server/dao/attributes/BaseAttributesService.java @@ -18,6 +18,7 @@ package org.thingsboard.server.dao.attributes; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.context.annotation.Primary; import org.springframework.stereotype.Service; @@ -45,6 +46,9 @@ import static org.thingsboard.server.dao.attributes.AttributeUtils.validate; public class BaseAttributesService implements AttributesService { private final AttributesDao attributesDao; + @Value("${sql.attributes.value_no_xss_validation:false}") + private boolean valueNoXssValidation; + public BaseAttributesService(AttributesDao attributesDao) { this.attributesDao = attributesDao; } @@ -82,14 +86,14 @@ public class BaseAttributesService implements AttributesService { @Override public ListenableFuture save(TenantId tenantId, EntityId entityId, String scope, AttributeKvEntry attribute) { validate(entityId, scope); - AttributeUtils.validate(attribute); + AttributeUtils.validate(attribute, valueNoXssValidation); return attributesDao.save(tenantId, entityId, scope, attribute); } @Override public ListenableFuture> save(TenantId tenantId, EntityId entityId, String scope, List attributes) { validate(entityId, scope); - AttributeUtils.validate(attributes); + AttributeUtils.validate(attributes, valueNoXssValidation); List> saveFutures = attributes.stream().map(attribute -> attributesDao.save(tenantId, entityId, scope, attribute)).collect(Collectors.toList()); return Futures.allAsList(saveFutures); } diff --git a/dao/src/main/java/org/thingsboard/server/dao/attributes/CachedAttributesService.java b/dao/src/main/java/org/thingsboard/server/dao/attributes/CachedAttributesService.java index b95ce39d9a..faff81670b 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/attributes/CachedAttributesService.java +++ b/dao/src/main/java/org/thingsboard/server/dao/attributes/CachedAttributesService.java @@ -69,6 +69,8 @@ public class CachedAttributesService implements AttributesService { @Value("${cache.type:caffeine}") private String cacheType; + @Value("${sql.attributes.value_no_xss_validation:false}") + private boolean valueNoXssValidation; public CachedAttributesService(AttributesDao attributesDao, StatsFactory statsFactory, @@ -212,7 +214,7 @@ public class CachedAttributesService implements AttributesService { @Override public ListenableFuture save(TenantId tenantId, EntityId entityId, String scope, AttributeKvEntry attribute) { validate(entityId, scope); - AttributeUtils.validate(attribute); + AttributeUtils.validate(attribute, valueNoXssValidation); ListenableFuture future = attributesDao.save(tenantId, entityId, scope, attribute); return Futures.transform(future, key -> evict(entityId, scope, attribute, key), cacheExecutor); } @@ -220,7 +222,7 @@ public class CachedAttributesService implements AttributesService { @Override public ListenableFuture> save(TenantId tenantId, EntityId entityId, String scope, List attributes) { validate(entityId, scope); - AttributeUtils.validate(attributes); + AttributeUtils.validate(attributes, valueNoXssValidation); List> futures = new ArrayList<>(attributes.size()); for (var attribute : attributes) { diff --git a/dao/src/main/java/org/thingsboard/server/dao/util/KvUtils.java b/dao/src/main/java/org/thingsboard/server/dao/util/KvUtils.java index 788a19228b..e417a5a50a 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/util/KvUtils.java +++ b/dao/src/main/java/org/thingsboard/server/dao/util/KvUtils.java @@ -15,6 +15,7 @@ */ package org.thingsboard.server.dao.util; +import com.fasterxml.jackson.databind.JsonNode; import com.github.benmanes.caffeine.cache.Cache; import com.github.benmanes.caffeine.cache.Caffeine; import org.thingsboard.server.common.data.kv.KvEntry; @@ -36,11 +37,11 @@ public class KvUtils { .maximumSize(100000).build(); } - public static void validate(List tsKvEntries) { - tsKvEntries.forEach(KvUtils::validate); + public static void validate(List tsKvEntries, boolean valueNoXssValidation) { + tsKvEntries.forEach(tsKvEntry -> validate(tsKvEntry, valueNoXssValidation)); } - public static void validate(KvEntry tsKvEntry) { + public static void validate(KvEntry tsKvEntry, boolean valueNoXssValidation) { if (tsKvEntry == null) { throw new IncorrectParameterException("Key value entry can't be null"); } @@ -55,14 +56,20 @@ public class KvUtils { throw new DataValidationException("Validation error: key length must be equal or less than 255"); } - if (validatedKeys.getIfPresent(key) != null) { - return; + if (validatedKeys.getIfPresent(key) == null) { + if (!NoXssValidator.isValid(key)) { + throw new DataValidationException("Validation error: key is malformed"); + } + validatedKeys.put(key, Boolean.TRUE); } - if (!NoXssValidator.isValid(key)) { - throw new DataValidationException("Validation error: key is malformed"); + if (valueNoXssValidation) { + Object value = tsKvEntry.getValue(); + if (value instanceof CharSequence || value instanceof JsonNode) { + if (!NoXssValidator.isValid(value.toString())) { + throw new DataValidationException("Validation error: value is malformed"); + } + } } - - validatedKeys.put(key, Boolean.TRUE); } }