From 222360905daac2a4d4dd396d005213c63ce51a53 Mon Sep 17 00:00:00 2001 From: dashevchenko Date: Thu, 30 Apr 2026 15:57:41 +0300 Subject: [PATCH 1/2] fixed gateway docker-compose.yml YAML Injection, enhanced device credential validation --- .../DeviceCredentialsDataValidator.java | 22 +++ .../dao/util/DeviceConnectivityUtil.java | 13 +- .../DeviceCredentialsDataValidatorTest.java | 128 ++++++++++++++++++ .../dao/util/DeviceConnectivityUtilTest.java | 90 ++++++++++++ 4 files changed, 249 insertions(+), 4 deletions(-) create mode 100644 dao/src/test/java/org/thingsboard/server/dao/service/validator/DeviceCredentialsDataValidatorTest.java diff --git a/dao/src/main/java/org/thingsboard/server/dao/service/validator/DeviceCredentialsDataValidator.java b/dao/src/main/java/org/thingsboard/server/dao/service/validator/DeviceCredentialsDataValidator.java index 7035fcfd9b..c8762a5b75 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/service/validator/DeviceCredentialsDataValidator.java +++ b/dao/src/main/java/org/thingsboard/server/dao/service/validator/DeviceCredentialsDataValidator.java @@ -18,18 +18,25 @@ package org.thingsboard.server.dao.service.validator; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Lazy; import org.springframework.stereotype.Component; +import org.thingsboard.common.util.JacksonUtil; import org.thingsboard.server.common.data.Device; import org.thingsboard.server.common.data.StringUtils; +import org.thingsboard.server.common.data.device.credentials.BasicMqttCredentials; import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.data.security.DeviceCredentials; +import org.thingsboard.server.common.data.security.DeviceCredentialsType; import org.thingsboard.server.dao.device.DeviceCredentialsDao; import org.thingsboard.server.dao.device.DeviceService; import org.thingsboard.server.dao.exception.DeviceCredentialsValidationException; import org.thingsboard.server.dao.service.DataValidator; +import java.util.regex.Pattern; + @Component public class DeviceCredentialsDataValidator extends DataValidator { + private static final Pattern CONTROL_CHARS = Pattern.compile("[\\r\\n\\t\\x00-\\x1F\\x7F]"); + @Autowired private DeviceCredentialsDao deviceCredentialsDao; @@ -69,9 +76,24 @@ public class DeviceCredentialsDataValidator extends DataValidator validator.validateDataImpl(tenantId, creds)) + .isInstanceOf(DeviceCredentialsValidationException.class) + .hasMessageContaining("credentialsId") + .hasMessageContaining("control characters"); + } + + @Test + void rejectsCarriageReturnInAccessToken() { + DeviceCredentials creds = accessToken("token\rprivileged: true"); + + assertThatThrownBy(() -> validator.validateDataImpl(tenantId, creds)) + .isInstanceOf(DeviceCredentialsValidationException.class) + .hasMessageContaining("control characters"); + } + + @Test + void rejectsNewlineInMqttClientId() { + DeviceCredentials creds = mqttBasic("cid\nentrypoint: x", "user", "pwd"); + + assertThatThrownBy(() -> validator.validateDataImpl(tenantId, creds)) + .isInstanceOf(DeviceCredentialsValidationException.class) + .hasMessageContaining("clientId"); + } + + @Test + void rejectsNewlineInMqttUserName() { + DeviceCredentials creds = mqttBasic("cid", "user\nprivileged: true", "pwd"); + + assertThatThrownBy(() -> validator.validateDataImpl(tenantId, creds)) + .isInstanceOf(DeviceCredentialsValidationException.class) + .hasMessageContaining("userName"); + } + + @Test + void rejectsNewlineInMqttPassword() { + DeviceCredentials creds = mqttBasic("cid", "user", "pwd\nentrypoint: x"); + + assertThatThrownBy(() -> validator.validateDataImpl(tenantId, creds)) + .isInstanceOf(DeviceCredentialsValidationException.class) + .hasMessageContaining("password"); + } + + @Test + void acceptsValidCredentials() { + willReturn(new Device()).given(deviceService).findDeviceById(tenantId, deviceId); + DeviceCredentials creds = accessToken("safe_token_123"); + + assertThatCode(() -> validator.validateDataImpl(tenantId, creds)) + .doesNotThrowAnyException(); + } + + private DeviceCredentials accessToken(String token) { + DeviceCredentials c = new DeviceCredentials(); + c.setDeviceId(deviceId); + c.setCredentialsType(DeviceCredentialsType.ACCESS_TOKEN); + c.setCredentialsId(token); + return c; + } + + private DeviceCredentials mqttBasic(String clientId, String userName, String password) { + BasicMqttCredentials inner = new BasicMqttCredentials(); + inner.setClientId(clientId); + inner.setUserName(userName); + inner.setPassword(password); + DeviceCredentials c = new DeviceCredentials(); + c.setDeviceId(deviceId); + c.setCredentialsType(DeviceCredentialsType.MQTT_BASIC); + c.setCredentialsId("mqtt-credentials-id"); + c.setCredentialsValue(JacksonUtil.toString(inner)); + return c; + } + +} diff --git a/dao/src/test/java/org/thingsboard/server/dao/util/DeviceConnectivityUtilTest.java b/dao/src/test/java/org/thingsboard/server/dao/util/DeviceConnectivityUtilTest.java index d4c93a1d5f..b69fefeea0 100644 --- a/dao/src/test/java/org/thingsboard/server/dao/util/DeviceConnectivityUtilTest.java +++ b/dao/src/test/java/org/thingsboard/server/dao/util/DeviceConnectivityUtilTest.java @@ -16,6 +16,13 @@ package org.thingsboard.server.dao.util; import org.junit.jupiter.api.Test; +import org.thingsboard.common.util.JacksonUtil; +import org.thingsboard.server.common.data.device.credentials.BasicMqttCredentials; +import org.thingsboard.server.common.data.security.DeviceCredentials; +import org.thingsboard.server.common.data.security.DeviceCredentialsType; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; import static org.assertj.core.api.Assertions.assertThat; @@ -29,4 +36,87 @@ class DeviceConnectivityUtilTest { assertThat(DeviceConnectivityUtil.CA_ROOT_CERT_PEM).doesNotContainAnyWhitespaces(); } + @Test + void validAccessTokenIsRenderedAsIs() throws Exception { + String yaml = renderCompose(accessToken("safe_token_123")); + + assertThat(yaml).contains("- TB_GW_ACCESS_TOKEN=safe_token_123\n"); + assertNoInjectedSiblingKeys(yaml); + } + + @Test + void newlineInAccessTokenIsSanitized() throws Exception { + String malicious = "safe_token\n entrypoint: [\"/bin/bash\",\"-c\",\"id\"]"; + + String yaml = renderCompose(accessToken(malicious)); + + assertNoInjectedSiblingKeys(yaml); + } + + @Test + void carriageReturnInAccessTokenIsSanitized() throws Exception { + String yaml = renderCompose(accessToken("token\rprivileged: true")); + + assertNoInjectedSiblingKeys(yaml); + } + + @Test + void newlineInMqttClientIdIsSanitized() throws Exception { + String yaml = renderCompose(mqttBasic("cid\n entrypoint: [\"/bin/sh\"]", "user", "pwd")); + + assertNoInjectedSiblingKeys(yaml); + } + + @Test + void newlineInMqttUserNameIsSanitized() throws Exception { + String yaml = renderCompose(mqttBasic("cid", "user\n privileged: true", "pwd")); + + assertNoInjectedSiblingKeys(yaml); + } + + @Test + void newlineInMqttPasswordIsSanitized() throws Exception { + String yaml = renderCompose(mqttBasic("cid", "user", "pwd\n entrypoint: [\"/bin/sh\"]")); + + assertNoInjectedSiblingKeys(yaml); + } + + private static String renderCompose(DeviceCredentials credentials) throws Exception { + var resource = DeviceConnectivityUtil.getGatewayDockerComposeFile( + "host.docker.internal", "3.8-stable", credentials); + try (var in = resource.getInputStream()) { + return new String(in.readAllBytes(), StandardCharsets.UTF_8); + } + } + + private static DeviceCredentials accessToken(String token) { + DeviceCredentials c = new DeviceCredentials(); + c.setCredentialsType(DeviceCredentialsType.ACCESS_TOKEN); + c.setCredentialsId(token); + return c; + } + + private static DeviceCredentials mqttBasic(String clientId, String userName, String password) { + BasicMqttCredentials inner = new BasicMqttCredentials(); + inner.setClientId(clientId); + inner.setUserName(userName); + inner.setPassword(password); + DeviceCredentials c = new DeviceCredentials(); + c.setCredentialsType(DeviceCredentialsType.MQTT_BASIC); + c.setCredentialsId("mqtt-credentials-id"); + c.setCredentialsValue(JacksonUtil.toString(inner)); + return c; + } + + private static void assertNoInjectedSiblingKeys(String yaml) throws IOException { + for (String line : yaml.split("\n")) { + String trimmed = line.replaceFirst("^\\s+", ""); + assertThat(trimmed) + .as("unexpected sibling key — possible YAML injection: %s", line) + .doesNotStartWith("entrypoint:") + .doesNotStartWith("privileged:") + .doesNotStartWith("command:"); + } + } + } From 157c773fcdc447795e56c0443a810df8ef23820c Mon Sep 17 00:00:00 2001 From: dashevchenko Date: Thu, 30 Apr 2026 17:19:19 +0300 Subject: [PATCH 2/2] smplified CONTROL_CHARS regex --- .../validator/DeviceCredentialsDataValidator.java | 2 +- .../server/dao/util/DeviceConnectivityUtil.java | 2 +- .../DeviceCredentialsDataValidatorTest.java | 15 ++++++++------- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/dao/src/main/java/org/thingsboard/server/dao/service/validator/DeviceCredentialsDataValidator.java b/dao/src/main/java/org/thingsboard/server/dao/service/validator/DeviceCredentialsDataValidator.java index c8762a5b75..96ffa613c7 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/service/validator/DeviceCredentialsDataValidator.java +++ b/dao/src/main/java/org/thingsboard/server/dao/service/validator/DeviceCredentialsDataValidator.java @@ -35,7 +35,7 @@ import java.util.regex.Pattern; @Component public class DeviceCredentialsDataValidator extends DataValidator { - private static final Pattern CONTROL_CHARS = Pattern.compile("[\\r\\n\\t\\x00-\\x1F\\x7F]"); + private static final Pattern CONTROL_CHARS = Pattern.compile("[\\x00-\\x1F\\x7F]"); @Autowired private DeviceCredentialsDao deviceCredentialsDao; diff --git a/dao/src/main/java/org/thingsboard/server/dao/util/DeviceConnectivityUtil.java b/dao/src/main/java/org/thingsboard/server/dao/util/DeviceConnectivityUtil.java index 707be1d8c2..a546023f42 100644 --- a/dao/src/main/java/org/thingsboard/server/dao/util/DeviceConnectivityUtil.java +++ b/dao/src/main/java/org/thingsboard/server/dao/util/DeviceConnectivityUtil.java @@ -50,7 +50,7 @@ public class DeviceConnectivityUtil { public static final String MQTT_IMAGE = "thingsboard/mosquitto-clients "; public static final String COAP_IMAGE = "thingsboard/coap-clients "; private final static Pattern VALID_URL_PATTERN = Pattern.compile("^(https?)://[-a-zA-Z0-9+&@#/%?=~_|!:,.;]*[-a-zA-Z0-9+&@#/%=~_|]"); - private final static Pattern CONTROL_CHARS = Pattern.compile("[\\r\\n\\t\\x00-\\x1F\\x7F]"); + private final static Pattern CONTROL_CHARS = Pattern.compile("[\\x00-\\x1F\\x7F]"); private static String sanitize(String value) { return value == null ? null : CONTROL_CHARS.matcher(value).replaceAll("_"); diff --git a/dao/src/test/java/org/thingsboard/server/dao/service/validator/DeviceCredentialsDataValidatorTest.java b/dao/src/test/java/org/thingsboard/server/dao/service/validator/DeviceCredentialsDataValidatorTest.java index c1b65fab76..2e43f9d102 100644 --- a/dao/src/test/java/org/thingsboard/server/dao/service/validator/DeviceCredentialsDataValidatorTest.java +++ b/dao/src/test/java/org/thingsboard/server/dao/service/validator/DeviceCredentialsDataValidatorTest.java @@ -16,9 +16,10 @@ package org.thingsboard.server.dao.service.validator; import org.junit.jupiter.api.Test; -import org.springframework.boot.test.context.SpringBootTest; -import org.springframework.boot.test.mock.mockito.MockBean; -import org.springframework.boot.test.mock.mockito.SpyBean; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; import org.thingsboard.common.util.JacksonUtil; import org.thingsboard.server.common.data.Device; import org.thingsboard.server.common.data.device.credentials.BasicMqttCredentials; @@ -36,14 +37,14 @@ import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.BDDMockito.willReturn; -@SpringBootTest(classes = DeviceCredentialsDataValidator.class) +@ExtendWith(MockitoExtension.class) class DeviceCredentialsDataValidatorTest { - @MockBean + @Mock DeviceCredentialsDao deviceCredentialsDao; - @MockBean + @Mock DeviceService deviceService; - @SpyBean + @InjectMocks DeviceCredentialsDataValidator validator; final TenantId tenantId = TenantId.fromUUID(UUID.fromString("9ef79cdf-37a8-4119-b682-2e7ed4e018da"));