Skip to content
Permalink
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
oranagra committed Oct 4, 2021
1 parent bb7597f commit 5674b00
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 7 deletions.
17 changes: 17 additions & 0 deletions src/networking.c
Expand Up @@ -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));

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
9 changes: 2 additions & 7 deletions src/server.c
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions src/server.h
Expand Up @@ -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, ...)
Expand Down
16 changes: 16 additions & 0 deletions tests/unit/auth.tcl
Expand Up @@ -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
}
}

0 comments on commit 5674b00

Please sign in to comment.