118 lines
4.6 KiB
Diff
118 lines
4.6 KiB
Diff
From 5674b0057ff2903d43eaff802017eddf37c360f8 Mon Sep 17 00:00:00 2001
|
|
From: Oran Agra <oran@redislabs.com>
|
|
Date: Wed, 9 Jun 2021 17:31:39 +0300
|
|
Subject: [PATCH] Prevent unauthenticated client from easily consuming lots of
|
|
memory (CVE-2021-32675)
|
|
|
|
This change sets a low limit for multibulk and bulk length in the
|
|
protocol for unauthenticated connections, so that they can't easily
|
|
cause redis to allocate massive amounts of memory by sending just a few
|
|
characters on the network.
|
|
The new limits are 10 arguments of 16kb each (instead of 1m of 512mb)
|
|
---
|
|
src/networking.c | 17 +++++++++++++++++
|
|
src/server.c | 9 ++-------
|
|
src/server.h | 1 +
|
|
tests/unit/auth.tcl | 16 ++++++++++++++++
|
|
4 files changed, 36 insertions(+), 7 deletions(-)
|
|
|
|
diff --git a/src/networking.c b/src/networking.c
|
|
index a61678dab2b..b02397c96f4 100644
|
|
--- a/src/networking.c
|
|
+++ b/src/networking.c
|
|
@@ -97,6 +97,15 @@ void linkClient(client *c) {
|
|
raxInsert(server.clients_index,(unsigned char*)&id,sizeof(id),c,NULL);
|
|
}
|
|
|
|
+int authRequired(client *c) {
|
|
+ /* Check if the user is authenticated. This check is skipped in case
|
|
+ * the default user is flagged as "nopass" and is active. */
|
|
+ int auth_required = (!(DefaultUser->flags & USER_FLAG_NOPASS) ||
|
|
+ (DefaultUser->flags & USER_FLAG_DISABLED)) &&
|
|
+ !c->authenticated;
|
|
+ return auth_required;
|
|
+}
|
|
+
|
|
client *createClient(connection *conn) {
|
|
client *c = zmalloc(sizeof(client));
|
|
|
|
@@ -1744,6 +1753,10 @@ int processMultibulkBuffer(client *c) {
|
|
addReplyError(c,"Protocol error: invalid multibulk length");
|
|
setProtocolError("invalid mbulk count",c);
|
|
return C_ERR;
|
|
+ } else if (ll > 10 && authRequired(c)) {
|
|
+ addReplyError(c, "Protocol error: unauthenticated multibulk length");
|
|
+ setProtocolError("unauth mbulk count", c);
|
|
+ return C_ERR;
|
|
}
|
|
|
|
c->qb_pos = (newline-c->querybuf)+2;
|
|
@@ -1791,6 +1804,10 @@ int processMultibulkBuffer(client *c) {
|
|
addReplyError(c,"Protocol error: invalid bulk length");
|
|
setProtocolError("invalid bulk length",c);
|
|
return C_ERR;
|
|
+ } else if (ll > 16384 && authRequired(c)) {
|
|
+ addReplyError(c, "Protocol error: unauthenticated bulk length");
|
|
+ setProtocolError("unauth bulk length", c);
|
|
+ return C_ERR;
|
|
}
|
|
|
|
c->qb_pos = newline-c->querybuf+2;
|
|
diff --git a/src/server.c b/src/server.c
|
|
index 93148f8e3ed..c8768b1824b 100644
|
|
--- a/src/server.c
|
|
+++ b/src/server.c
|
|
@@ -3590,13 +3590,8 @@ int processCommand(client *c) {
|
|
int is_denyloading_command = !(c->cmd->flags & CMD_LOADING) ||
|
|
(c->cmd->proc == execCommand && (c->mstate.cmd_inv_flags & CMD_LOADING));
|
|
|
|
- /* Check if the user is authenticated. This check is skipped in case
|
|
- * the default user is flagged as "nopass" and is active. */
|
|
- int auth_required = (!(DefaultUser->flags & USER_FLAG_NOPASS) ||
|
|
- (DefaultUser->flags & USER_FLAG_DISABLED)) &&
|
|
- !c->authenticated;
|
|
- if (auth_required) {
|
|
- /* AUTH and HELLO and no auth modules are valid even in
|
|
+ if (authRequired(c)) {
|
|
+ /* AUTH and HELLO and no auth commands are valid even in
|
|
* non-authenticated state. */
|
|
if (!(c->cmd->flags & CMD_NO_AUTH)) {
|
|
rejectCommand(c,shared.noautherr);
|
|
diff --git a/src/server.h b/src/server.h
|
|
index a16f2885829..530355421a8 100644
|
|
--- a/src/server.h
|
|
+++ b/src/server.h
|
|
@@ -1743,6 +1743,7 @@ void protectClient(client *c);
|
|
void unprotectClient(client *c);
|
|
void initThreadedIO(void);
|
|
client *lookupClientByID(uint64_t id);
|
|
+int authRequired(client *c);
|
|
|
|
#ifdef __GNUC__
|
|
void addReplyErrorFormat(client *c, const char *fmt, ...)
|
|
diff --git a/tests/unit/auth.tcl b/tests/unit/auth.tcl
|
|
index 9080d4bf7e9..530ca7c8d91 100644
|
|
--- a/tests/unit/auth.tcl
|
|
+++ b/tests/unit/auth.tcl
|
|
@@ -24,4 +24,20 @@ start_server {tags {"auth"} overrides {requirepass foobar}} {
|
|
r set foo 100
|
|
r incr foo
|
|
} {101}
|
|
+
|
|
+ test {For unauthenticated clients multibulk and bulk length are limited} {
|
|
+ set rr [redis [srv "host"] [srv "port"] 0 $::tls]
|
|
+ $rr write "*100\r\n"
|
|
+ $rr flush
|
|
+ catch {[$rr read]} e
|
|
+ assert_match {*unauthenticated multibulk length*} $e
|
|
+ $rr close
|
|
+
|
|
+ set rr [redis [srv "host"] [srv "port"] 0 $::tls]
|
|
+ $rr write "*1\r\n\$100000000\r\n"
|
|
+ $rr flush
|
|
+ catch {[$rr read]} e
|
|
+ assert_match {*unauthenticated bulk length*} $e
|
|
+ $rr close
|
|
+ }
|
|
}
|