1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
|
From a71a65e9ed75b347c33bc882b38f4f1006fcba39 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)
CVE: CVE-2021-32675
Upstream-Status: Backport[https://github.com/redis/redis/commit/5674b0057ff2903d43eaff802017eddf37c360f8]
Signed-off-by: Changqing Li <changqing.li@windriver.com>
---
src/networking.c | 17 +++++++++++++++++
src/server.c | 11 +++--------
src/server.h | 1 +
tests/unit/auth.tcl | 16 ++++++++++++++++
4 files changed, 37 insertions(+), 8 deletions(-)
diff --git a/src/networking.c b/src/networking.c
index 2355a37..8e891c6 100644
--- a/src/networking.c
+++ b/src/networking.c
@@ -107,6 +107,15 @@ static void clientSetDefaultAuth(client *c) {
!(c->user->flags & USER_FLAG_DISABLED);
}
+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));
@@ -1855,6 +1864,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;
@@ -1902,6 +1915,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 9932606..f65ad22 100644
--- a/src/server.c
+++ b/src/server.c
@@ -3996,14 +3996,9 @@ int processCommand(client *c) {
int is_may_replicate_command = (c->cmd->flags & (CMD_WRITE | CMD_MAY_REPLICATE)) ||
(c->cmd->proc == execCommand && (c->mstate.cmd_flags & (CMD_WRITE | CMD_MAY_REPLICATE)));
- /* 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
- * non-authenticated state. */
+ 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);
return C_OK;
diff --git a/src/server.h b/src/server.h
index e256ce0..a3dfe60 100644
--- a/src/server.h
+++ b/src/server.h
@@ -1894,6 +1894,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 b63cf01..5997707 100644
--- a/tests/unit/auth.tcl
+++ b/tests/unit/auth.tcl
@@ -24,6 +24,22 @@ 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
+ }
}
start_server {tags {"auth_binary_password"}} {
--
2.17.1
|
