From bfa493d4be6b390c1b0915971b42cb98418f0c78 Mon Sep 17 00:00:00 2001 From: Aurelien DARRAGON Date: Mon, 20 Jan 2025 10:42:42 +0100 Subject: [PATCH] BUG/MAJOR: log/sink: possible sink collision in sink_new_from_srv() sink_new_from_srv() leverages sink_new_buf() with the server id as name, sink_new_buf() then calls __sink_new() with the provided name. Unfortunately sink_new() is designed in such a way that it will first look up in the list of existing sinks to check if a sink already exists with given name, in which case the existing sink is returned. While this behavior may be error-prone, it is actually up to the caller to ensure that the provided name is unique if it really expects a unique sink pointer. Due to this bug in sink_new_from_srv(), multiple tcp servers with the same name defined in distinct log backends would end up sharing the same sink, which means messages sent to one of the servers would also be forwarded to all servers with the same name across all log backend sections defined in the config, which is obviously an issue and could even raise security concerns. Example: defaults log backend@log-1 local0 backend log-1 mode log server s1 127.0.0.1:514 backend log-2 mode log server s1 127.0.0.1:5114 With the above config, logs sent to log-1/s1 would also end up being sent to log-2/s1 due to server id "s1" being used for tcp servers in distinct log backends. To fix the issue, we now prefix the sink ame with the backend name: back_name/srv_id combination is known to be unique (backend name serves as a namespace) This bug was reported by GH user @landon-lengyel under #2846. UDP servers (with udp@ prefix before the address) are not affected as they don't make use of the sink facility. As a workaround, one should manually ensure that all tcp servers across different log backends (backend with "mode log" enabled) use unique names This bug was introduced in e58a9b4 ("MINOR: sink: add sink_new_from_srv() function") thus it exists since the introduction of log backends in 2.9, which means this patch should be backported up to 2.9. --- src/sink.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/sink.c b/src/sink.c index b86c336bc..25d7a2fee 100644 --- a/src/sink.c +++ b/src/sink.c @@ -1285,17 +1285,28 @@ struct sink *sink_new_from_srv(struct server *srv, const char *from) { struct sink *sink = NULL; int bufsize = (srv->log_bufsize) ? srv->log_bufsize : BUFSIZE; + char *sink_name = NULL; /* prepare description for the sink */ chunk_reset(&trash); chunk_printf(&trash, "created from %s declared into '%s' at line %d", from, srv->conf.file, srv->conf.line); + memprintf(&sink_name, "%s/%s", srv->proxy->id, srv->id); + if (!sink_name) { + ha_alert("memory error while creating ring buffer for server '%s/%s'.\n", srv->proxy->id, srv->id); + goto error; + } + /* directly create a sink of BUF type, and use UNSPEC log format to * inherit from caller fmt in sink_write() + * + * sink_name must be unique to prevent existing sink from being re-used */ - sink = sink_new_buf(srv->id, trash.area, LOG_FORMAT_UNSPEC, bufsize); + sink = sink_new_buf(sink_name, trash.area, LOG_FORMAT_UNSPEC, bufsize); + ha_free(&sink_name); // no longer needed + if (!sink) { - ha_alert("unable to create a new sink buffer for server '%s'.\n", srv->id); + ha_alert("unable to create a new sink buffer for server '%s/%s'.\n", srv->proxy->id, srv->id); goto error; }