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)
This commit is contained in:
Oran Agra 2021-06-09 17:31:39 +03:00
parent 04ba485042
commit 757f8f771e
4 changed files with 36 additions and 7 deletions

View File

@ -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));
@ -1860,6 +1869,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;
@ -1907,6 +1920,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;

View File

@ -4001,13 +4001,8 @@ 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
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);

View File

@ -1897,6 +1897,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, ...)

View File

@ -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"}} {