Merge pull request #8353 from thingsboard/fix/noxss-kv

Fixed xss vulnerabilities in attributes and telemetry

application/src/main/java/org/thingsboard/server/controller/TelemetryController.java

@@ -75,16 +75,15 @@ import org.thingsboard.server.common.data.kv.StringDataEntry;
 import org.thingsboard.server.common.data.kv.TsKvEntry;
 import org.thingsboard.server.common.data.tenant.profile.DefaultTenantProfileConfiguration;
 import org.thingsboard.server.common.transport.adaptor.JsonConverter;
-import org.thingsboard.server.dao.service.ConstraintValidator;
 import org.thingsboard.server.dao.timeseries.TimeseriesService;
+import org.thingsboard.server.exception.InvalidParametersException;
+import org.thingsboard.server.exception.UncheckedApiException;
 import org.thingsboard.server.queue.util.TbCoreComponent;
 import org.thingsboard.server.service.security.AccessValidator;
 import org.thingsboard.server.service.security.model.SecurityUser;
 import org.thingsboard.server.service.security.permission.Operation;
 import org.thingsboard.server.service.telemetry.AttributeData;
 import org.thingsboard.server.service.telemetry.TsData;
-import org.thingsboard.server.exception.InvalidParametersException;
-import org.thingsboard.server.exception.UncheckedApiException;
 
 import javax.annotation.Nullable;
 import javax.annotation.PostConstruct;
@@ -624,7 +623,6 @@ public class TelemetryController extends BaseController {
         }
         if (json.isObject()) {
             List<AttributeKvEntry> attributes = extractRequestAttributes(json);
-            attributes.forEach(ConstraintValidator::validateFields);
             if (attributes.isEmpty()) {
                 return getImmediateDeferredResult("No attributes data found in request body!", HttpStatus.BAD_REQUEST);
             }

application/src/main/java/org/thingsboard/server/service/security/AccessValidator.java

@@ -67,6 +67,7 @@ import org.thingsboard.server.dao.device.DeviceProfileService;
 import org.thingsboard.server.dao.device.DeviceService;
 import org.thingsboard.server.dao.edge.EdgeService;
 import org.thingsboard.server.dao.entityview.EntityViewService;
+import org.thingsboard.server.dao.exception.DataValidationException;
 import org.thingsboard.server.dao.exception.IncorrectParameterException;
 import org.thingsboard.server.dao.ota.OtaPackageService;
 import org.thingsboard.server.dao.resource.ResourceService;
@@ -575,7 +576,7 @@ public class AccessValidator {
         ResponseEntity responseEntity;
         if (e instanceof ToErrorResponseEntity) {
             responseEntity = ((ToErrorResponseEntity) e).toErrorResponseEntity();
-        } else if (e instanceof IllegalArgumentException || e instanceof IncorrectParameterException) {
+        } else if (e instanceof IllegalArgumentException || e instanceof IncorrectParameterException || e instanceof DataValidationException) {
             responseEntity = new ResponseEntity<>(e.getMessage(), HttpStatus.BAD_REQUEST);
         } else {
             responseEntity = new ResponseEntity<>(defaultErrorStatus);

application/src/main/resources/thingsboard.yml

@@ -259,11 +259,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
+    noxss_validation_enabled: "${SQL_ATTRIBUTES_NOXSS_VALIDATION_ENABLED:true}"
   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
+    noxss_validation_enabled: "${SQL_TS_NOXSS_VALIDATION_ENABLED:true}"
   ts_latest:
     batch_size: "${SQL_TS_LATEST_BATCH_SIZE:10000}"
     batch_max_delay: "${SQL_TS_LATEST_BATCH_MAX_DELAY_MS:100}"

application/src/test/java/org/thingsboard/server/controller/BaseTelemetryControllerTest.java

@@ -0,0 +1,58 @@
+/**
+ * Copyright © 2016-2023 The Thingsboard Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.thingsboard.server.controller;
+
+import org.junit.Test;
+import org.thingsboard.server.common.data.Device;
+import org.thingsboard.server.common.data.SaveDeviceWithCredentialsRequest;
+import org.thingsboard.server.common.data.security.DeviceCredentials;
+import org.thingsboard.server.common.data.security.DeviceCredentialsType;
+
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
+
+public abstract class BaseTelemetryControllerTest extends AbstractControllerTest {
+
+    @Test
+    public void testConstraintValidator() 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\": \"<object data=\\\"data:text/html,<script>alert(document)</script>\\\"></object>\"}";
+        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());
+        invalidRequestBody = "{\"<object data=\\\"data:text/html,<script>alert(document)</script>\\\"></object>\": \"data\"}";
+        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";
+
+        Device device = new Device();
+        device.setName("My device");
+        device.setType("default");
+
+        DeviceCredentials deviceCredentials = new DeviceCredentials();
+        deviceCredentials.setCredentialsType(DeviceCredentialsType.ACCESS_TOKEN);
+        deviceCredentials.setCredentialsId(testToken);
+
+        SaveDeviceWithCredentialsRequest saveRequest = new SaveDeviceWithCredentialsRequest(device, deviceCredentials);
+
+        return readResponse(doPost("/api/device-with-credentials", saveRequest).andExpect(status().isOk()), Device.class);
+    }
+}

application/src/test/java/org/thingsboard/server/controller/sql/TelemetryControllerSqlTest.java

@@ -0,0 +1,23 @@
+/**
+ * Copyright © 2016-2023 The Thingsboard Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.thingsboard.server.controller.sql;
+
+import org.thingsboard.server.controller.BaseTelemetryControllerTest;
+import org.thingsboard.server.dao.service.DaoSqlTest;
+
+@DaoSqlTest
+public class TelemetryControllerSqlTest extends BaseTelemetryControllerTest {
+}

common/data/src/main/java/org/thingsboard/server/common/data/kv/BasicKvEntry.java

@@ -16,6 +16,7 @@
 package org.thingsboard.server.common.data.kv;
 
 import org.thingsboard.server.common.data.validation.Length;
+import org.thingsboard.server.common.data.validation.NoXss;
 
 import java.util.Objects;
 import java.util.Optional;
@@ -23,6 +24,7 @@ import java.util.Optional;
 public abstract class BasicKvEntry implements KvEntry {
 
     @Length(fieldName = "attribute key")
+    @NoXss
     private final String key;
 
     protected BasicKvEntry(String key) {

common/data/src/main/java/org/thingsboard/server/common/data/kv/BasicTsKvEntry.java

@@ -15,12 +15,14 @@
  */
 package org.thingsboard.server.common.data.kv;
 
+import javax.validation.Valid;
 import java.util.Objects;
 import java.util.Optional;
 
 public class BasicTsKvEntry implements TsKvEntry {
     private static final int MAX_CHARS_PER_DATA_POINT = 512;
     protected final long ts;
+    @Valid
     private final KvEntry kv;
 
     public BasicTsKvEntry(long ts, KvEntry kv) {

common/data/src/main/java/org/thingsboard/server/common/data/kv/JsonDataEntry.java

@@ -15,10 +15,14 @@
  */
 package org.thingsboard.server.common.data.kv;
 
+import org.thingsboard.server.common.data.validation.NoXss;
+
 import java.util.Objects;
 import java.util.Optional;
 
 public class JsonDataEntry extends BasicKvEntry {
+
+    @NoXss
     private final String value;
 
     public JsonDataEntry(String key, String value) {

common/data/src/main/java/org/thingsboard/server/common/data/kv/StringDataEntry.java

@@ -15,6 +15,8 @@
  */
 package org.thingsboard.server.common.data.kv;
 
+import org.thingsboard.server.common.data.validation.NoXss;
+
 import java.util.Objects;
 import java.util.Optional;
 
@@ -22,6 +24,7 @@ public class StringDataEntry extends BasicKvEntry {
 
     private static final long serialVersionUID = 1L;
 
+    @NoXss
     private final String value;
 
     public StringDataEntry(String key, String value) {

dao/src/main/java/org/thingsboard/server/dao/attributes/AttributeUtils.java

@@ -19,6 +19,9 @@ import org.thingsboard.server.common.data.id.EntityId;
 import org.thingsboard.server.common.data.kv.AttributeKvEntry;
 import org.thingsboard.server.dao.exception.IncorrectParameterException;
 import org.thingsboard.server.dao.service.Validator;
+import org.thingsboard.server.dao.util.KvUtils;
+
+import java.util.List;
 
 public class AttributeUtils {
 
@@ -27,10 +30,13 @@ public class AttributeUtils {
         Validator.validateString(scope, "Incorrect scope " + scope);
     }
 
-    public static void validate(AttributeKvEntry kvEntry) {
+    public static void validate(List<AttributeKvEntry> kvEntries, boolean validateNoxss) {
-        if (kvEntry == null) {
+        kvEntries.forEach(kv -> validate(kv, validateNoxss));
-            throw new IncorrectParameterException("Key value entry can't be null");
+    }
-        } else if (kvEntry.getDataType() == null) {
+
+    public static void validate(AttributeKvEntry kvEntry, boolean validateNoxss) {
+        KvUtils.validate(kvEntry, validateNoxss);
+        if (kvEntry.getDataType() == null) {
             throw new IncorrectParameterException("Incorrect kvEntry. Data type can't be null");
         } else {
             Validator.validateString(kvEntry.getKey(), "Incorrect kvEntry. Key can't be empty");

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.noxss_validation_enabled:true}")
+    private boolean noxssValidationEnabled;
+
     public BaseAttributesService(AttributesDao attributesDao) {
         this.attributesDao = attributesDao;
     }
@@ -82,14 +86,14 @@ public class BaseAttributesService implements AttributesService {
     @Override
     public ListenableFuture<String> save(TenantId tenantId, EntityId entityId, String scope, AttributeKvEntry attribute) {
         validate(entityId, scope);
-        AttributeUtils.validate(attribute);
+        AttributeUtils.validate(attribute, noxssValidationEnabled);
         return attributesDao.save(tenantId, entityId, scope, attribute);
     }
 
     @Override
     public ListenableFuture<List<String>> save(TenantId tenantId, EntityId entityId, String scope, List<AttributeKvEntry> attributes) {
         validate(entityId, scope);
-        attributes.forEach(AttributeUtils::validate);
+        AttributeUtils.validate(attributes, noxssValidationEnabled);
         List<ListenableFuture<String>> saveFutures = attributes.stream().map(attribute -> attributesDao.save(tenantId, entityId, scope, attribute)).collect(Collectors.toList());
         return Futures.allAsList(saveFutures);
     }

dao/src/main/java/org/thingsboard/server/dao/attributes/CachedAttributesService.java

@@ -70,6 +70,9 @@ public class CachedAttributesService implements AttributesService {
     @Value("${cache.type:caffeine}")
     private String cacheType;
 
+    @Value("${sql.attributes.noxss_validation_enabled:true}")
+    private boolean noxssValidationEnabled;
+
     public CachedAttributesService(AttributesDao attributesDao,
                                    StatsFactory statsFactory,
                                    CacheExecutorService cacheExecutorService,
@@ -212,7 +215,7 @@ public class CachedAttributesService implements AttributesService {
     @Override
     public ListenableFuture<String> save(TenantId tenantId, EntityId entityId, String scope, AttributeKvEntry attribute) {
         validate(entityId, scope);
-        AttributeUtils.validate(attribute);
+        AttributeUtils.validate(attribute, noxssValidationEnabled);
         ListenableFuture<String> future = attributesDao.save(tenantId, entityId, scope, attribute);
         return Futures.transform(future, key -> evict(entityId, scope, attribute, key), cacheExecutor);
     }
@@ -220,7 +223,7 @@ public class CachedAttributesService implements AttributesService {
     @Override
     public ListenableFuture<List<String>> save(TenantId tenantId, EntityId entityId, String scope, List<AttributeKvEntry> attributes) {
         validate(entityId, scope);
-        attributes.forEach(AttributeUtils::validate);
+        AttributeUtils.validate(attributes, noxssValidationEnabled);
 
         List<ListenableFuture<String>> futures = new ArrayList<>(attributes.size());
         for (var attribute : attributes) {

dao/src/main/java/org/thingsboard/server/dao/timeseries/BaseTimeseriesService.java

@@ -42,6 +42,7 @@ import org.thingsboard.server.common.data.kv.TsKvLatestRemovingResult;
 import org.thingsboard.server.dao.entityview.EntityViewService;
 import org.thingsboard.server.dao.exception.IncorrectParameterException;
 import org.thingsboard.server.dao.service.Validator;
+import org.thingsboard.server.dao.util.KvUtils;
 
 import java.util.Collection;
 import java.util.Collections;
@@ -80,6 +81,9 @@ public class BaseTimeseriesService implements TimeseriesService {
     @Value("${database.ts_max_intervals}")
     private long maxTsIntervals;
 
+    @Value("${sql.ts.noxss_validation_enabled:true}")
+    private boolean noxssValidationEnabled;
+
     @Autowired
     private TimeseriesDao timeseriesDao;
 
@@ -155,10 +159,8 @@ public class BaseTimeseriesService implements TimeseriesService {
 
     @Override
     public ListenableFuture<Integer> save(TenantId tenantId, EntityId entityId, TsKvEntry tsKvEntry) {
+        KvUtils.validate(tsKvEntry, noxssValidationEnabled);
         validate(entityId);
-        if (tsKvEntry == null) {
-            throw new IncorrectParameterException("Key value entry can't be null");
-        }
         List<ListenableFuture<Integer>> futures = Lists.newArrayListWithExpectedSize(INSERTS_PER_ENTRY);
         saveAndRegisterFutures(tenantId, futures, entityId, tsKvEntry, 0L);
         return Futures.transform(Futures.allAsList(futures), SUM_ALL_INTEGERS, MoreExecutors.directExecutor());
@@ -175,12 +177,10 @@ public class BaseTimeseriesService implements TimeseriesService {
     }
 
     private ListenableFuture<Integer> doSave(TenantId tenantId, EntityId entityId, List<TsKvEntry> tsKvEntries, long ttl, boolean saveLatest) {
+        KvUtils.validate(tsKvEntries, noxssValidationEnabled);
         int inserts = saveLatest ? INSERTS_PER_ENTRY : INSERTS_PER_ENTRY_WITHOUT_LATEST;
         List<ListenableFuture<Integer>> futures = Lists.newArrayListWithExpectedSize(tsKvEntries.size() * inserts);
         for (TsKvEntry tsKvEntry : tsKvEntries) {
-            if (tsKvEntry == null) {
-                throw new IncorrectParameterException("Key value entry can't be null");
-            }
             if (saveLatest) {
                 saveAndRegisterFutures(tenantId, futures, entityId, tsKvEntry, ttl);
             } else {
@@ -192,11 +192,9 @@ public class BaseTimeseriesService implements TimeseriesService {
 
     @Override
     public ListenableFuture<List<Void>> saveLatest(TenantId tenantId, EntityId entityId, List<TsKvEntry> tsKvEntries) {
+        KvUtils.validate(tsKvEntries, noxssValidationEnabled);
         List<ListenableFuture<Void>> futures = Lists.newArrayListWithExpectedSize(tsKvEntries.size());
         for (TsKvEntry tsKvEntry : tsKvEntries) {
-            if (tsKvEntry == null) {
-                throw new IncorrectParameterException("Key value entry can't be null");
-            }
             futures.add(timeseriesLatestDao.saveLatest(tenantId, entityId, tsKvEntry));
         }
         return Futures.allAsList(futures);

dao/src/main/java/org/thingsboard/server/dao/util/KvUtils.java

@@ -0,0 +1,37 @@
+/**
+ * Copyright © 2016-2023 The Thingsboard Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.thingsboard.server.dao.util;
+
+import org.thingsboard.server.common.data.kv.KvEntry;
+import org.thingsboard.server.dao.exception.IncorrectParameterException;
+import org.thingsboard.server.dao.service.ConstraintValidator;
+
+import java.util.List;
+
+public class KvUtils {
+    public static void validate(List<? extends KvEntry> tsKvEntries, boolean validateNoxss) {
+        tsKvEntries.forEach(kvEntry -> validate(kvEntry, validateNoxss));
+    }
+
+    public static void validate(KvEntry tsKvEntry, boolean validateNoxss) {
+        if (tsKvEntry == null) {
+            throw new IncorrectParameterException("Key value entry can't be null");
+        }
+        if (validateNoxss) {
+            ConstraintValidator.validateFields(tsKvEntry);
+        }
+    }
+}

dao/src/test/java/org/thingsboard/server/dao/service/ConstraintValidatorTest.java

@@ -0,0 +1,57 @@
+/**
+ * Copyright © 2016-2023 The Thingsboard Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.thingsboard.server.dao.service;
+
+import org.junit.Assert;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.thingsboard.server.common.data.kv.JsonDataEntry;
+import org.thingsboard.server.common.data.kv.StringDataEntry;
+import org.thingsboard.server.dao.exception.DataValidationException;
+
+class ConstraintValidatorTest {
+
+    private static final int MIN_IN_MS = 60000;
+    private static final int _1M = 1_000_000;
+
+    @Test
+    void validateFields() {
+        StringDataEntry stringDataEntryValid = new StringDataEntry("key", "value");
+
+        StringDataEntry stringDataEntryInvalid1 = new StringDataEntry("<object type=\"text/html\"><script>alert(document)</script></object>", "value");
+        StringDataEntry stringDataEntryInvalid2 = new StringDataEntry("key", "<object type=\"text/html\"><script>alert(document)</script></object>");
+
+        JsonDataEntry jsonDataEntryInvalid = new JsonDataEntry("key", "{\"value\": <object type=\"text/html\"><script>alert(document)</script></object>}");
+
+        Assert.assertThrows(DataValidationException.class, () -> ConstraintValidator.validateFields(stringDataEntryInvalid1));
+        Assert.assertThrows(DataValidationException.class, () -> ConstraintValidator.validateFields(stringDataEntryInvalid2));
+        Assert.assertThrows(DataValidationException.class, () -> ConstraintValidator.validateFields(jsonDataEntryInvalid));
+        ConstraintValidator.validateFields(stringDataEntryValid);
+    }
+
+    @Test
+    void validatePerMinute() {
+        StringDataEntry stringDataEntryValid = new StringDataEntry("key", "value");
+
+        long start = System.currentTimeMillis();
+        for (int i = 0; i < _1M; i++) {
+            ConstraintValidator.validateFields(stringDataEntryValid);
+        }
+        long end = System.currentTimeMillis();
+
+        Assertions.assertTrue(MIN_IN_MS > end - start);
+    }
+}