Skip to content

Security hardening: sanitize MQTT topics, fix bounds check, reduce lo… #14

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mplogas merged 3 commits into from
May 3, 2026
Merged

Security hardening: sanitize MQTT topics, fix bounds check, reduce lo… #14

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3db733a
Security hardening: sanitize MQTT topics, fix bounds check, reduce lo…
mplogas Mar 13, 2026
994eb87
Address PR review: anchor-relative topic parsing, remove Sanitize, ad…
mplogas Mar 19, 2026
1a8fac4
chore: solution folder added
mplogas Apr 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 0 additions & 21 deletions src/Ecowitt.Controller/Dockerfile
View file
Open in desktop

This file was deleted.

4 changes: 0 additions & 4 deletions src/Ecowitt.Controller/Ecowitt.Controller.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="FluentValidation.DependencyInjectionExtensions" Version="12.1.1" />
<PackageReference Include="Microsoft.Extensions.Http.Polly" Version="10.0.3" />
<PackageReference Include="Microsoft.VisualStudio.Azure.Containers.Tools.Targets" Version="1.23.0" />
<PackageReference Include="MQTTnet" Version="5.1.0.1559" />
Expand All @@ -36,11 +35,8 @@
<PackageReference Include="SlimMessageBus" Version="3.0.0" />
<PackageReference Include="SlimMessageBus.Host" Version="3.3.6" />
<PackageReference Include="SlimMessageBus.Host.AspNetCore" Version="3.3.6" />
<PackageReference Include="SlimMessageBus.Host.FluentValidation" Version="3.3.6" />
<PackageReference Include="SlimMessageBus.Host.Memory" Version="3.3.6" />
<PackageReference Include="SlimMessageBus.Host.Serialization.Json" Version="3.3.6" />
<PackageReference Include="Swashbuckle.AspNetCore" Version="10.1.4" />
</ItemGroup>


</Project>
33 changes: 21 additions & 12 deletions src/Ecowitt.Controller/Service/Mqtt/MqttPathBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,57 +4,66 @@ public static class MqttPathBuilder
{
public static string BuildMqttGatewayTopic(string gwName)
{
return Sanitize(gwName);
return SanitizeSegment(gwName);
}

public static string BuildMqttGatewaySensorTopic(string gwName, string sensorName)
{
return Sanitize($"{gwName}/sensors/{sensorName}");
return $"{SanitizeSegment(gwName)}/sensors/{SanitizeSegment(sensorName)}";
}

public static string BuildMqttGatewayDiagnosticTopic(string gwName, string sensorName)
{
return Sanitize($"{gwName}/diag/{sensorName}");
return $"{SanitizeSegment(gwName)}/diag/{SanitizeSegment(sensorName)}";
}

public static string BuildMqttSubdeviceTopic(string gwName, string subdeviceName)
{
return Sanitize($"{gwName}/subdevices/{subdeviceName}");
return $"{SanitizeSegment(gwName)}/subdevices/{SanitizeSegment(subdeviceName)}";
}

public static string BuildMqttSubdeviceSensorTopic(string gwName, string subdeviceName, string sensorName)
{
return Sanitize($"{gwName}/subdevices/{subdeviceName}/sensors/{sensorName}");
return $"{SanitizeSegment(gwName)}/subdevices/{SanitizeSegment(subdeviceName)}/sensors/{SanitizeSegment(sensorName)}";
}


public static string BuildMqttSubdeviceDiagnosticTopic(string gwName, string subdeviceName, string sensorName)
{
return Sanitize($"{gwName}/subdevices/{subdeviceName}/diag/{sensorName}");
return $"{SanitizeSegment(gwName)}/subdevices/{SanitizeSegment(subdeviceName)}/diag/{SanitizeSegment(sensorName)}";
}

public static string BuildMqttSubdeviceCommandTopic(string gwName, string subdeviceName)
{
return Sanitize($"{gwName}/subdevices/{subdeviceName}/cmd");
return $"{SanitizeSegment(gwName)}/subdevices/{SanitizeSegment(subdeviceName)}/cmd";
}

public static string BuildMqttSubdeviceCommandTopic()
{
return Sanitize($"+/subdevices/+/cmd");
return "+/subdevices/+/cmd";
}

public static string BuildMqttSubdeviceHACommandTopic(string gwName, string subdeviceName)
{
return Sanitize($"{gwName}/subdevices/{subdeviceName}/cmd/homeassistant");
return $"{SanitizeSegment(gwName)}/subdevices/{SanitizeSegment(subdeviceName)}/cmd/homeassistant";
}

public static string BuildMqttSubdeviceHACommandTopic()
{
return Sanitize($"+/subdevices/+/cmd/homeassistant");
return "+/subdevices/+/cmd/homeassistant";
}

public static string Sanitize(string input)
{
return input.Replace(' ', '-').ToLowerInvariant();
}
Comment thread
mplogas marked this conversation as resolved.
Outdated
}

public static string SanitizeSegment(string input)
{
return input
.Replace(' ', '-')
.Replace('/', '-')
.Replace('#', '-')
.Replace('+', '-')
.ToLowerInvariant();
Comment thread
mplogas marked this conversation as resolved.
}
}
2 changes: 1 addition & 1 deletion src/Ecowitt.Controller/Service/Mqtt/MqttService.Consumer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public async Task OnHandle(DiscoveryRemovalEvent message, CancellationToken canc
foreach (var sensor in message.Sensors)
{
var sensorClassTopic = BuildSensorClassTopic(sensor.SensorClass);
var topic = $"homeassistant/{MqttPathBuilder.Sanitize($"{sensorClassTopic}/{message.DeviceName}_{sensor.Name}")}/config";
var topic = $"homeassistant/{sensorClassTopic}/{MqttPathBuilder.SanitizeSegment($"{message.DeviceName}_{sensor.Name}")}/config";
await RemoveDiscoveryMessage(topic);
_logger.LogInformation("Removed discovery for {DeviceName}/{SensorName}", message.DeviceName, sensor.Name);
}
Expand Down
8 changes: 4 additions & 4 deletions src/Ecowitt.Controller/Service/Mqtt/MqttService.DiscoveryPublisher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ private async Task PublishGatewayDiscovery(Device gw)

var config = DiscoveryBuilder.BuildGatewayConfig(device, _origin, "Availability", id, availabilityTopic, availabilityTopic);

await PublishDiscoveryMessage("sensor", MqttPathBuilder.Sanitize($"sensor/{gw.Name}"), config);
await PublishDiscoveryMessage("sensor", $"sensor/{MqttPathBuilder.SanitizeSegment(gw.Name)}", config);
}

private async Task PublishSubdeviceDiscovery(Device gw, Ecowitt.Controller.Model.Subdevice subdevice)
Expand All @@ -33,7 +33,7 @@ private async Task PublishSubdeviceDiscovery(Device gw, Ecowitt.Controller.Model

var config = DiscoveryBuilder.BuildGatewayConfig(device, _origin, "Availability", id, availabilityTopic, availabilityTopic);

await PublishDiscoveryMessage("sensor", MqttPathBuilder.Sanitize($"sensor/{subdevice.Nickname}"), config);
await PublishDiscoveryMessage("sensor", $"sensor/{MqttPathBuilder.SanitizeSegment(subdevice.Nickname)}", config);
}

private async Task PublishSubdeviceSwitchDiscovery(Device gw, Ecowitt.Controller.Model.Subdevice subdevice)
Expand All @@ -47,7 +47,7 @@ private async Task PublishSubdeviceSwitchDiscovery(Device gw, Ecowitt.Controller
var config =
DiscoveryBuilder.BuildSwitchConfig(device, _origin, "switch", id, statetopic, cmdTopic, valueTemplate: valueTemplate);

await PublishDiscoveryMessage("switch", MqttPathBuilder.Sanitize($"switch/{subdevice.Nickname}"), config);
await PublishDiscoveryMessage("switch", $"switch/{MqttPathBuilder.SanitizeSegment(subdevice.Nickname)}", config);
}

private async Task PublishSensorDiscovery(Device gw, ISensor sensor)
Expand Down Expand Up @@ -83,7 +83,7 @@ private async Task PublishSensorDiscovery(Model.Discovery.Device device, ISensor

var sensorClassTopic = BuildSensorClassTopic(sensor.SensorClass);

await PublishDiscoveryMessage(sensorClassTopic, MqttPathBuilder.Sanitize($"{sensorClassTopic}/{device.Name}_{sensor.Name}"), config);
await PublishDiscoveryMessage(sensorClassTopic, $"{sensorClassTopic}/{MqttPathBuilder.SanitizeSegment($"{device.Name}_{sensor.Name}")}", config);
}

private async Task PublishDiscoveryMessage(string domain, string topic, Config config)
Expand Down
3 changes: 2 additions & 1 deletion src/Ecowitt.Controller/Service/Mqtt/MqttService.Events.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ private async Task ClientOnApplicationMessageReceivedAsync(MqttApplicationMessag
else if (topic.EndsWith("cmd/homeassistant"))
{
// commands coming from home assistant
if (int.TryParse(topic.Split('/')[3], out var result))
var parts = topic.Split('/');
if (parts.Length > 3 && int.TryParse(parts[3], out var result))
Comment thread
mplogas marked this conversation as resolved.
Outdated
{
var cmd = payload.Equals("ON", StringComparison.InvariantCultureIgnoreCase) ? Command.Start : Command.Stop; //I know, everything that's not "ON" is "OFF"
await _messageBus.Publish(new SubdeviceApiCommand() { Cmd = cmd, Id = result });
Expand Down
4 changes: 2 additions & 2 deletions src/Ecowitt.Controller/Service/Mqtt/MqttService.Publisher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ private async Task<bool> Publish(string topic, string message, bool retain = fal
_logger.LogDebug("Message {message} ", message);
return true;
}
else _logger.LogWarning("Failed to publish message {Message} to topic {Topic}. Reason: {MqttClientPublishReasonCode}", message, topic, result.ReasonCode);
else _logger.LogWarning("Failed to publish message to topic {Topic}. Reason: {MqttClientPublishReasonCode}", topic, result.ReasonCode);
}
else _logger.LogWarning("Can't publish message {Message} to topic {Topic}. Client not connected.", message, topic);
else _logger.LogWarning("Can't publish message to topic {Topic}. Client not connected.", topic);
return false;
}
}
Expand Down
Loading