mirror of
https://github.com/Cockatrice/Cockatrice.git
synced 2026-11-05 09:33:25 -08:00
[Security] Harden network input handling and websocket proxying (#7076)
Some checks are pending
CodeQL / Analyze (cpp) (push) Waiting to run
CodeQL / Analyze (actions) (push) Waiting to run
Build Desktop / Configure (push) Waiting to run
Build Desktop / Debian 13 (push) Blocked by required conditions
Build Desktop / Debian 12 (push) Blocked by required conditions
Build Desktop / Fedora 44 (push) Blocked by required conditions
Build Desktop / Fedora 43 (push) Blocked by required conditions
Build Desktop / Servatrice_Debian 12 (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 (push) Blocked by required conditions
Build Desktop / Ubuntu 24.04 (push) Blocked by required conditions
Build Desktop / Arch (push) Blocked by required conditions
Build Desktop / macOS 13 Intel (push) Blocked by required conditions
Build Desktop / macOS 14 (push) Blocked by required conditions
Build Desktop / macOS 15 (push) Blocked by required conditions
Build Desktop / macOS 26 Debug (push) Blocked by required conditions
Build Desktop / Windows 10 (push) Blocked by required conditions
Build Docker / Servatrice (arm) (push) Waiting to run
Build Docker / Servatrice (x86) (push) Waiting to run
Build Docker / Publish multi-platform Servatrice image (push) Blocked by required conditions
Some checks are pending
CodeQL / Analyze (cpp) (push) Waiting to run
CodeQL / Analyze (actions) (push) Waiting to run
Build Desktop / Configure (push) Waiting to run
Build Desktop / Debian 13 (push) Blocked by required conditions
Build Desktop / Debian 12 (push) Blocked by required conditions
Build Desktop / Fedora 44 (push) Blocked by required conditions
Build Desktop / Fedora 43 (push) Blocked by required conditions
Build Desktop / Servatrice_Debian 12 (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 (push) Blocked by required conditions
Build Desktop / Ubuntu 24.04 (push) Blocked by required conditions
Build Desktop / Arch (push) Blocked by required conditions
Build Desktop / macOS 13 Intel (push) Blocked by required conditions
Build Desktop / macOS 14 (push) Blocked by required conditions
Build Desktop / macOS 15 (push) Blocked by required conditions
Build Desktop / macOS 26 Debug (push) Blocked by required conditions
Build Desktop / Windows 10 (push) Blocked by required conditions
Build Docker / Servatrice (arm) (push) Waiting to run
Build Docker / Servatrice (x86) (push) Waiting to run
Build Docker / Publish multi-platform Servatrice image (push) Blocked by required conditions
- TCP client connections: reject messages whose declared length is negative or exceeds a 1.5MB cap immediately instead of buffering indefinitely, preventing unbounded memory growth and the previously stuck state caused by negative lengths overflowing the int header. - ISL connections: apply the same length validation with a 4MB cap, disconnecting the peer on an implausible length. - Websockets: only trust the forwarded client-address header (e.g. X-Forwarded-For) when the websocket connection originates from an address in security/trusted_sources. Previously any client could spoof its address to bypass bans, user limits and rate limiting. Took 3 minutes Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
This commit is contained in:
parent
514014dc00
commit
d0fc27e3a8
3 changed files with 37 additions and 5 deletions
|
|
@ -181,6 +181,9 @@ inline Q_LOGGING_CATEGORY(WebsocketServerSocketInterfaceLog, "websocket_server_s
|
|||
|
||||
static const int protocolVersion = 14;
|
||||
|
||||
// Maximum size of a single TCP protocol message. Matches the websocket limit.
|
||||
static const int MAX_TCP_MESSAGE_SIZE = 1500000;
|
||||
|
||||
AbstractServerSocketInterface::AbstractServerSocketInterface(Servatrice *_server,
|
||||
Servatrice_DatabaseInterface *_databaseInterface,
|
||||
QObject *parent)
|
||||
|
|
@ -4226,13 +4229,21 @@ void TcpServerSocketInterface::readClient()
|
|||
(((quint32)(unsigned char)inputBuffer[1]) << 16) +
|
||||
(((quint32)(unsigned char)inputBuffer[2]) << 8) +
|
||||
((quint32)(unsigned char)inputBuffer[3]);
|
||||
// Reject implausible lengths instead of buffering until they arrive.
|
||||
// This also handles lengths that overflow into negative int values.
|
||||
if (messageLength < 0 || messageLength > MAX_TCP_MESSAGE_SIZE) {
|
||||
qCWarning(TcpServerSocketInterfaceLog)
|
||||
<< "Invalid message length" << messageLength << "from" << getAddress();
|
||||
prepareDestroy();
|
||||
return;
|
||||
}
|
||||
inputBuffer.remove(0, 4);
|
||||
messageInProgress = true;
|
||||
} else {
|
||||
return;
|
||||
}
|
||||
}
|
||||
if (inputBuffer.size() < messageLength || messageLength < 0) {
|
||||
if (inputBuffer.size() < messageLength) {
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -4338,10 +4349,15 @@ void WebsocketServerSocketInterface::initConnection(void *_socket)
|
|||
|
||||
QByteArray websocketIPHeader = settingsCache->value("server/web_socket_ip_header", "").toByteArray();
|
||||
if (websocketIPHeader.length() > 0 && socket->request().hasRawHeader(websocketIPHeader)) {
|
||||
QString header(socket->request().rawHeader(websocketIPHeader));
|
||||
QHostAddress parsed(header);
|
||||
if (!parsed.isNull()) {
|
||||
address = parsed;
|
||||
// Only trust the forwarded header if the connection itself comes from a trusted proxy,
|
||||
// otherwise a client could spoof its address to bypass bans and rate limits.
|
||||
QString trustedSources = settingsCache->value("security/trusted_sources", "127.0.0.1,::1").toString();
|
||||
if (trustedSources.contains(socket->peerAddress().toString(), Qt::CaseInsensitive)) {
|
||||
QString header(socket->request().rawHeader(websocketIPHeader));
|
||||
QHostAddress parsed(header);
|
||||
if (!parsed.isNull()) {
|
||||
address = parsed;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue