mirror of
https://github.com/Cockatrice/Cockatrice.git
synced 2026-08-22 00:34:28 -05:00
[Security] Harden network input handling and websocket proxying
- 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
This commit is contained in:
@@ -41,6 +41,12 @@ websocket_host=any
|
||||
; The TCP port number servatrice will listen on for websockets clients; default is 4748
|
||||
websocket_port=4748
|
||||
|
||||
; If websockets are served through a reverse proxy (e.g. nginx), the real client address can be
|
||||
; read from an HTTP header such as X-Forwarded-For. The header is ONLY trusted when the websocket
|
||||
; connection itself originates from an address listed in security/trusted_sources (default 127.0.0.1,::1),
|
||||
; so that clients cannot spoof their address to bypass bans or rate limits.
|
||||
web_socket_ip_header=
|
||||
|
||||
; When database is enabled, servatrice writes the server status in the "update" database table; this
|
||||
; setting defines every how many milliseconds servatrice will update its status; default is 15000 (15 secs)
|
||||
statusupdate=15000
|
||||
|
||||
@@ -22,6 +22,9 @@
|
||||
#include <server_protocolhandler.h>
|
||||
#include <server_room.h>
|
||||
|
||||
// Maximum size of a single ISL message.
|
||||
static const int MAX_ISL_MESSAGE_SIZE = 4 * 1024 * 1024;
|
||||
|
||||
inline Q_LOGGING_CATEGORY(IslInterfaceLog, "isl_interface");
|
||||
|
||||
void IslInterface::sharedCtor(const QSslCertificate &cert, const QSslKey &privateKey)
|
||||
@@ -243,6 +246,13 @@ void IslInterface::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_ISL_MESSAGE_SIZE) {
|
||||
qCWarning(IslInterfaceLog) << "Invalid message length" << messageLength << "from" << peerAddress;
|
||||
socket->disconnectFromHost();
|
||||
return;
|
||||
}
|
||||
inputBuffer.remove(0, 4);
|
||||
messageInProgress = true;
|
||||
} else {
|
||||
|
||||
@@ -94,6 +94,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)
|
||||
@@ -2306,13 +2309,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;
|
||||
}
|
||||
|
||||
@@ -2418,10 +2429,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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user