fixed pr comments

common/data/src/main/java/org/thingsboard/server/common/data/StringUtils.java

@@ -24,6 +24,7 @@ import java.util.Arrays;
 import java.util.Base64;
 import java.util.List;
 import java.util.function.Function;
+import java.util.regex.Pattern;
 
 import static org.apache.commons.lang3.StringUtils.repeat;
 
@@ -37,6 +38,12 @@ public class StringUtils {
 
     public static final int INDEX_NOT_FOUND = -1;
 
+    public static final Pattern CONTROL_CHARS = Pattern.compile("[\\x00-\\x1F\\x7F]");
+
+    public static boolean containsControlChars(String source) {
+        return source != null && CONTROL_CHARS.matcher(source).find();
+    }
+
     public static boolean isEmpty(String source) {
         return source == null || source.isEmpty();
     }

dao/src/main/java/org/thingsboard/server/dao/service/validator/DeviceCredentialsDataValidator.java

@@ -30,13 +30,9 @@ 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<DeviceCredentials> {
 
-    private static final Pattern CONTROL_CHARS = Pattern.compile("[\\x00-\\x1F\\x7F]");
-
     @Autowired
     private DeviceCredentialsDao deviceCredentialsDao;
 
@@ -92,7 +88,7 @@ public class DeviceCredentialsDataValidator extends DataValidator<DeviceCredenti
     }
 
     private static void rejectControlChars(String value, String fieldName) {
-        if (value != null && CONTROL_CHARS.matcher(value).find()) {
+        if (StringUtils.containsControlChars(value)) {
             throw new DeviceCredentialsValidationException(fieldName + " must not contain control characters!");
         }
     }

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

@@ -50,15 +50,28 @@ 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("[\\x00-\\x1F\\x7F]");
 
-    private static String sanitize(String value) {
-        return value == null ? null : CONTROL_CHARS.matcher(value).replaceAll("_");
+    private static String stripControlChars(String value) {
+        return value == null ? null : StringUtils.CONTROL_CHARS.matcher(value).replaceAll("_");
+    }
+
+    // Escapes a value that is interpolated inside a double-quoted shell argument (e.g. -u "...") in the
+    // publish commands shown to the operator, so it cannot break out of the quotes or trigger command/
+    // variable substitution. Control chars are stripped first to keep the command on a single line.
+    private static String escapeShellArg(String value) {
+        if (value == null) {
+            return null;
+        }
+        return stripControlChars(value)
+                .replace("\\", "\\\\")
+                .replace("\"", "\\\"")
+                .replace("$", "\\$")
+                .replace("`", "\\`");
     }
 
     public static String getHttpPublishCommand(String protocol, String host, String port, DeviceCredentials deviceCredentials) {
         return String.format("curl -v -X POST %s://%s%s/api/v1/%s/telemetry --header Content-Type:application/json --data " + JSON_EXAMPLE_PAYLOAD,
-                protocol, host, port, deviceCredentials.getCredentialsId());
+                protocol, host, port, stripControlChars(deviceCredentials.getCredentialsId()));
     }
 
     public static String getMqttPublishCommand(String protocol, String host, String port, String deviceTelemetryTopic, DeviceCredentials deviceCredentials) {
@@ -71,20 +84,20 @@ public class DeviceConnectivityUtil {
 
         switch (deviceCredentials.getCredentialsType()) {
             case ACCESS_TOKEN:
-                command.append(" -u \"").append(deviceCredentials.getCredentialsId()).append("\"");
+                command.append(" -u \"").append(escapeShellArg(deviceCredentials.getCredentialsId())).append("\"");
                 break;
             case MQTT_BASIC:
                 BasicMqttCredentials credentials = JacksonUtil.fromString(deviceCredentials.getCredentialsValue(),
                         BasicMqttCredentials.class);
                 if (credentials != null) {
                     if (StringUtils.isNotEmpty(credentials.getClientId())) {
-                        command.append(" -i \"").append(credentials.getClientId()).append("\"");
+                        command.append(" -i \"").append(escapeShellArg(credentials.getClientId())).append("\"");
                     }
                     if (StringUtils.isNotEmpty(credentials.getUserName())) {
-                        command.append(" -u \"").append(credentials.getUserName()).append("\"");
+                        command.append(" -u \"").append(escapeShellArg(credentials.getUserName())).append("\"");
                     }
                     if (StringUtils.isNotEmpty(credentials.getPassword())) {
-                        command.append(" -P \"").append(credentials.getPassword()).append("\"");
+                        command.append(" -P \"").append(escapeShellArg(credentials.getPassword())).append("\"");
                     }
                 } else {
                     return null;
@@ -122,12 +135,12 @@ public class DeviceConnectivityUtil {
         dockerComposeBuilder.append("\n");
         dockerComposeBuilder.append("    # Environment variables\n");
         dockerComposeBuilder.append("    environment:\n");
-        dockerComposeBuilder.append("      - TB_GW_HOST=").append(isLocalhost(host) ? HOST_DOCKER_INTERNAL : host).append("\n");
+        dockerComposeBuilder.append("      - TB_GW_HOST=").append(stripControlChars(isLocalhost(host) ? HOST_DOCKER_INTERNAL : host)).append("\n");
         dockerComposeBuilder.append("      - TB_GW_PORT=1883\n");
         switch (deviceCredentials.getCredentialsType()) {
             case ACCESS_TOKEN:
                 dockerComposeBuilder.append("      - TB_GW_SECURITY_TYPE=accessToken\n");
-                dockerComposeBuilder.append("      - TB_GW_ACCESS_TOKEN=").append(sanitize(deviceCredentials.getCredentialsId())).append("\n");
+                dockerComposeBuilder.append("      - TB_GW_ACCESS_TOKEN=").append(stripControlChars(deviceCredentials.getCredentialsId())).append("\n");
                 break;
             case MQTT_BASIC:
                 dockerComposeBuilder.append("      - TB_GW_SECURITY_TYPE=usernamePassword\n");
@@ -135,13 +148,13 @@ public class DeviceConnectivityUtil {
                         BasicMqttCredentials.class);
                 if (credentials != null) {
                     if (StringUtils.isNotEmpty(credentials.getClientId())) {
-                        dockerComposeBuilder.append("      - TB_GW_CLIENT_ID=").append(sanitize(credentials.getClientId())).append("\n");
+                        dockerComposeBuilder.append("      - TB_GW_CLIENT_ID=").append(stripControlChars(credentials.getClientId())).append("\n");
                     }
                     if (StringUtils.isNotEmpty(credentials.getUserName())) {
-                        dockerComposeBuilder.append("      - TB_GW_USERNAME=").append(sanitize(credentials.getUserName())).append("\n");
+                        dockerComposeBuilder.append("      - TB_GW_USERNAME=").append(stripControlChars(credentials.getUserName())).append("\n");
                     }
                     if (StringUtils.isNotEmpty(credentials.getPassword())) {
-                        dockerComposeBuilder.append("      - TB_GW_PASSWORD=").append(sanitize(credentials.getPassword())).append("\n");
+                        dockerComposeBuilder.append("      - TB_GW_PASSWORD=").append(stripControlChars(credentials.getPassword())).append("\n");
                     }
                 }
                 break;
@@ -206,7 +219,7 @@ public class DeviceConnectivityUtil {
                 String client = COAPS.equals(protocol) ? "coap-client-openssl" : "coap-client";
                 String certificate = COAPS.equals(protocol) ? " -R " + CA_ROOT_CERT_PEM : "";
                 return String.format("%s -v 6 -m POST%s -t \"application/json\" -e %s %s://%s%s/api/v1/%s/telemetry",
-                        client, certificate, JSON_EXAMPLE_PAYLOAD, protocol, host, port, deviceCredentials.getCredentialsId());
+                        client, certificate, JSON_EXAMPLE_PAYLOAD, protocol, host, port, stripControlChars(deviceCredentials.getCredentialsId()));
             default:
                 return null;
         }

dao/src/test/java/org/thingsboard/server/dao/service/validator/DeviceCredentialsDataValidatorTest.java

@@ -105,6 +105,15 @@ class DeviceCredentialsDataValidatorTest {
                 .doesNotThrowAnyException();
     }
 
+    @Test
+    void acceptsValidMqttBasicCredentials() {
+        willReturn(new Device()).given(deviceService).findDeviceById(tenantId, deviceId);
+        DeviceCredentials creds = mqttBasic("client-1", "user-1", "pwd-1");
+
+        assertThatCode(() -> validator.validateDataImpl(tenantId, creds))
+                .doesNotThrowAnyException();
+    }
+
     private DeviceCredentials accessToken(String token) {
         DeviceCredentials c = new DeviceCredentials();
         c.setDeviceId(deviceId);

dao/src/test/java/org/thingsboard/server/dao/util/DeviceConnectivityUtilTest.java

@@ -81,6 +81,39 @@ class DeviceConnectivityUtilTest {
         assertNoInjectedSiblingKeys(yaml);
     }
 
+    @Test
+    void mqttBasicQuoteInUserNameIsEscapedInPublishCommand() {
+        String command = DeviceConnectivityUtil.getMqttPublishCommand(
+                "mqtt", "localhost", "1883", "v1/devices/me/telemetry",
+                mqttBasic("cid", "u\";touch pwned;echo \"", "pwd"));
+
+        // the double quote must be backslash-escaped so it cannot terminate the -u "..." argument
+        assertThat(command).contains("-u \"u\\\";touch pwned;echo \\\"\"");
+        assertThat(command).doesNotContain("-u \"u\";");
+    }
+
+    @Test
+    void controlCharsInMqttClientIdAreStrippedInPublishCommand() {
+        String command = DeviceConnectivityUtil.getMqttPublishCommand(
+                "mqtt", "localhost", "1883", "v1/devices/me/telemetry",
+                mqttBasic("c\nid", "user", "pwd"));
+
+        assertThat(command).doesNotContain("\n");
+        assertThat(command).contains("-i \"c_id\"");
+    }
+
+    @Test
+    void controlCharsInAccessTokenAreStrippedInHttpAndCoapCommands() {
+        DeviceCredentials creds = accessToken("tok\nen");
+
+        assertThat(DeviceConnectivityUtil.getHttpPublishCommand("http", "localhost", ":8080", creds))
+                .doesNotContain("\n")
+                .contains("/api/v1/tok_en/telemetry");
+        assertThat(DeviceConnectivityUtil.getCoapPublishCommand("coap", "localhost", ":5683", creds))
+                .doesNotContain("\n")
+                .contains("/api/v1/tok_en/telemetry");
+    }
+
     private static String renderCompose(DeviceCredentials credentials) throws Exception {
         var resource = DeviceConnectivityUtil.getGatewayDockerComposeFile(
                 "host.docker.internal", "3.8-stable", credentials);