Unverified Commit 9a2c6ba4 authored by debing.sun's avatar debing.sun Committed by GitHub
Browse files

Prevent negative expire parameter in HEXPIRE/HEXPIREAT/HPEXPIRE/HPEXPIREAT commands (#13310)



1. Don't allow HEXPIRE/HEXPIREAT/HPEXPIRE/HPEXPIREAT command expire
parameters is negative

2. Remove a dead code reported from Coverity.
when `unit` is not `UNIT_SECONDS`, the second `if (expire > (long long)
EB_EXPIRE_TIME_MAX)` will be dead code.
```c
# t_hash.c
2988    /* Check expire overflow */
      	cond_at_most: Condition expire > 281474976710655LL, taking false branch. Now the value of expire is at most 281474976710655.
2989    if (expire > (long long) EB_EXPIRE_TIME_MAX) {
2990        addReplyErrorExpireTime(c);
2991        return;
2992    }

2994    if (unit == UNIT_SECONDS) {
2995        if (expire > (long long) EB_EXPIRE_TIME_MAX / 1000) {
2996            addReplyErrorExpireTime(c);
2997            return;
2998        }
2999        expire *= 1000;
3000    } else {
      	at_most: At condition expire > 281474976710655LL, the value of expire must be at most 281474976710655.
      	dead_error_condition: The condition expire > 281474976710655LL cannot be true.
3001        if (expire > (long long) EB_EXPIRE_TIME_MAX) {
      	
CID 494223: (#1 of 1): Logically dead code (DEADCODE)
dead_error_begin: Execution cannot reach this statement: addReplyErrorExpireTime(c);.
3002            addReplyErrorExpireTime(c);
3003            return;
3004        }
3005    }
```

---------
Co-authored-by: default avatarOzan Tezcan <ozantezcan@gmail.com>
parent f36b5a85
...@@ -2869,7 +2869,7 @@ static void httlGenericCommand(client *c, const char *cmd, long long basetime, i ...@@ -2869,7 +2869,7 @@ static void httlGenericCommand(client *c, const char *cmd, long long basetime, i
/* Verify `numFields` is consistent with number of arguments */ /* Verify `numFields` is consistent with number of arguments */
if (numFields > (c->argc - numFieldsAt - 1)) { if (numFields > (c->argc - numFieldsAt - 1)) {
addReplyError(c, "Parameter `numFileds` is more than number of arguments"); addReplyError(c, "Parameter `numFields` is more than number of arguments");
return; return;
} }
...@@ -2983,10 +2983,8 @@ static void hexpireGenericCommand(client *c, const char *cmd, long long basetime ...@@ -2983,10 +2983,8 @@ static void hexpireGenericCommand(client *c, const char *cmd, long long basetime
/* Read the expiry time from command */ /* Read the expiry time from command */
if (getLongLongFromObjectOrReply(c, expireArg, &expire, NULL) != C_OK) if (getLongLongFromObjectOrReply(c, expireArg, &expire, NULL) != C_OK)
return; return;
if (expire < 0) {
/* Check expire overflow */ addReplyError(c,"invalid expire time, must be >= 0");
if (expire > (long long) EB_EXPIRE_TIME_MAX) {
addReplyErrorExpireTime(c);
return; return;
} }
...@@ -2996,13 +2994,9 @@ static void hexpireGenericCommand(client *c, const char *cmd, long long basetime ...@@ -2996,13 +2994,9 @@ static void hexpireGenericCommand(client *c, const char *cmd, long long basetime
return; return;
} }
expire *= 1000; expire *= 1000;
} else {
if (expire > (long long) EB_EXPIRE_TIME_MAX) {
addReplyErrorExpireTime(c);
return;
}
} }
/* Ensure that the final absolute Unix timestamp does not exceed EB_EXPIRE_TIME_MAX. */
if (expire > (long long) EB_EXPIRE_TIME_MAX - basetime) { if (expire > (long long) EB_EXPIRE_TIME_MAX - basetime) {
addReplyErrorExpireTime(c); addReplyErrorExpireTime(c);
return; return;
...@@ -3033,7 +3027,7 @@ static void hexpireGenericCommand(client *c, const char *cmd, long long basetime ...@@ -3033,7 +3027,7 @@ static void hexpireGenericCommand(client *c, const char *cmd, long long basetime
/* Verify `numFields` is consistent with number of arguments */ /* Verify `numFields` is consistent with number of arguments */
if (numFields > (c->argc - numFieldsAt - 1)) { if (numFields > (c->argc - numFieldsAt - 1)) {
addReplyError(c, "Parameter `numFileds` is more than number of arguments"); addReplyError(c, "Parameter `numFields` is more than number of arguments");
return; return;
} }
...@@ -3130,7 +3124,7 @@ void hpersistCommand(client *c) { ...@@ -3130,7 +3124,7 @@ void hpersistCommand(client *c) {
/* Verify `numFields` is consistent with number of arguments */ /* Verify `numFields` is consistent with number of arguments */
if (numFields > (c->argc - numFieldsAt - 1)) { if (numFields > (c->argc - numFieldsAt - 1)) {
addReplyError(c, "Parameter `numFileds` is more than number of arguments"); addReplyError(c, "Parameter `numFields` is more than number of arguments");
return; return;
} }
......
...@@ -128,6 +128,19 @@ start_server {tags {"external:skip needs:debug"}} { ...@@ -128,6 +128,19 @@ start_server {tags {"external:skip needs:debug"}} {
r readraw 0 r readraw 0
} }
test "HEXPIRE/HEXPIREAT/HPEXPIRE/HPEXPIREAT - Verify that the expire time does not overflow" {
r del myhash
r hset myhash f1 v1
# The expire time can't be negative.
assert_error {ERR invalid expire time, must be >= 0} {r HEXPIRE myhash -1 FIELDS 1 f1}
assert_error {ERR invalid expire time, must be >= 0} {r HEXPIRE myhash -9223372036854775808 FIELDS 1 f1}
# The expire time can't be greater than the EB_EXPIRE_TIME_MAX
assert_error {ERR invalid expire time in 'hexpire' command} {r HEXPIRE myhash [expr (1<<48) / 1000] FIELDS 1 f1}
assert_error {ERR invalid expire time in 'hexpireat' command} {r HEXPIREAT myhash [expr (1<<48) / 1000 + [clock seconds] + 100] FIELDS 1 f1}
assert_error {ERR invalid expire time in 'hpexpire' command} {r HPEXPIRE myhash [expr (1<<48)] FIELDS 1 f1}
assert_error {ERR invalid expire time in 'hpexpireat' command} {r HPEXPIREAT myhash [expr (1<<48) + [clock milliseconds] + 100] FIELDS 1 f1}
}
test "HPEXPIRE(AT) - Test 'NX' flag ($type)" { test "HPEXPIRE(AT) - Test 'NX' flag ($type)" {
r del myhash r del myhash
r hset myhash field1 value1 field2 value2 field3 value3 r hset myhash field1 value1 field2 value2 field3 value3
...@@ -192,7 +205,7 @@ start_server {tags {"external:skip needs:debug"}} { ...@@ -192,7 +205,7 @@ start_server {tags {"external:skip needs:debug"}} {
r del myhash r del myhash
r hset myhash f1 v1 r hset myhash f1 v1
assert_error {*Parameter `numFields` should be greater than 0} {r hpexpire myhash 1000 NX FIELDS 0 f1 f2 f3} assert_error {*Parameter `numFields` should be greater than 0} {r hpexpire myhash 1000 NX FIELDS 0 f1 f2 f3}
assert_error {*Parameter `numFileds` is more than number of arguments} {r hpexpire myhash 1000 NX FIELDS 4 f1 f2 f3} assert_error {*Parameter `numFields` is more than number of arguments} {r hpexpire myhash 1000 NX FIELDS 4 f1 f2 f3}
} }
test "HPEXPIRE - parameter expire-time near limit of 2^48 ($type)" { test "HPEXPIRE - parameter expire-time near limit of 2^48 ($type)" {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment