Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix EVAL crashing server when numkeys is -1 #1568

Merged
merged 2 commits into from
Jul 9, 2023

Conversation

enjoy-binbin
Copy link
Member

The check allow we passing -1 is wrong, without
this fix, passing a -1 will crash the server.

The check allow we passing -1 is wrong, without
this fix, passing a -1 will crash the server.
@enjoy-binbin
Copy link
Member Author

@git-hulk @PragmaTwice
this one need to pay attention, it may be a security issue, which will cause the server to crash directly:

127.0.0.1:6666> eval 'redis.call("ping")' -1
Error: Server closed the connection

[root@binblog incubator-kvrocks]# ./build/kvrocks --log-level info
WARNING: No config file specified, using the default configuration. In order to specify a config file use 'kvrocks -c /path/to/kvrocks.conf'
terminate called after throwing an instance of 'std::length_error'
  what():  cannot create std::vector larger than max_size()
E20230709 15:19:49.662325 32543 main.cc:84] ======= Ooops! kvrocks unstable (commit 26e9b99) got signal: Aborted (6) =======
E20230709 15:19:49.683763 32543 main.cc:101] /lib64/libpthread.so.0(+0xf5d0) [0x7f137c5405d0]
E20230709 15:19:49.688210 32543 main.cc:99] /lib64/libc.so.6(gsignal+0x37) [0x7f137be98207]   __GI_raise
E20230709 15:19:49.688387 32543 main.cc:99] /lib64/libc.so.6(abort+0x148) [0x7f137be998f8]    __GI_abort
E20230709 15:19:49.688431 32543 main.cc:99] ./build/kvrocks() [0x4918c1]                      (/root/kvrocks/incubator-kvrocks/build/kvrocks (deleted)+0x4918c1)
E20230709 15:19:49.688460 32543 main.cc:99] ./build/kvrocks() [0xd19e86]                      (/root/kvrocks/incubator-kvrocks/build/kvrocks (deleted)+0xd19e86)
E20230709 15:19:49.688491 32543 main.cc:99] ./build/kvrocks() [0xd19ef1]                      (/root/kvrocks/incubator-kvrocks/build/kvrocks (deleted)+0xd19ef1)
E20230709 15:19:49.688520 32543 main.cc:99] ./build/kvrocks() [0xd1a044]                      (/root/kvrocks/incubator-kvrocks/build/kvrocks (deleted)+0xd1a044)
E20230709 15:19:49.688546 32543 main.cc:99] ./build/kvrocks() [0x492e15]                      (/root/kvrocks/incubator-kvrocks/build/kvrocks (deleted)+0x492e15)
E20230709 15:19:49.688575 32543 main.cc:99] ./build/kvrocks() [0x52aef6]                      (/root/kvrocks/incubator-kvrocks/build/kvrocks (deleted)+0x52aef6)
E20230709 15:19:49.688602 32543 main.cc:99] ./build/kvrocks() [0x592031]                      (/root/kvrocks/incubator-kvrocks/build/kvrocks (deleted)+0x592031)
E20230709 15:19:49.688649 32543 main.cc:99] ./build/kvrocks() [0x592ac0]                      (/root/kvrocks/incubator-kvrocks/build/kvrocks (deleted)+0x592ac0)
E20230709 15:19:49.688678 32543 main.cc:99] ./build/kvrocks() [0xb920ee]                      (/root/kvrocks/incubator-kvrocks/build/kvrocks (deleted)+0xb920ee)
E20230709 15:19:49.688704 32543 main.cc:99] ./build/kvrocks() [0xb993ac]                      (/root/kvrocks/incubator-kvrocks/build/kvrocks (deleted)+0xb993ac)
E20230709 15:19:49.688730 32543 main.cc:99] ./build/kvrocks() [0xb99a9f]                      (/root/kvrocks/incubator-kvrocks/build/kvrocks (deleted)+0xb99a9f)
E20230709 15:19:49.688761 32543 main.cc:99] ./build/kvrocks() [0x5bb599]                      (/root/kvrocks/incubator-kvrocks/build/kvrocks (deleted)+0x5bb599)
E20230709 15:19:49.688863 32543 main.cc:99] ./build/kvrocks() [0xd95c60]                      (/root/kvrocks/incubator-kvrocks/build/kvrocks (deleted)+0xd95c60)
E20230709 15:19:49.688954 32543 main.cc:99] /lib64/libpthread.so.0(+0x7dd5) [0x7f137c538dd5]  start_thread
E20230709 15:19:49.689146 32543 main.cc:99] /lib64/libc.so.6(clone+0x6d) [0x7f137bf5fead]     __clone
Aborted

@git-hulk
Copy link
Member

git-hulk commented Jul 9, 2023

Thanks for your good catch. @enjoy-binbin

@PragmaTwice PragmaTwice added A-security area security A-scripting area scripting labels Jul 9, 2023
@PragmaTwice
Copy link
Member

Great catch! Thanks.

@PragmaTwice PragmaTwice merged commit 506f672 into apache:unstable Jul 9, 2023
@PragmaTwice
Copy link
Member

Thanks for your contribution!

@enjoy-binbin enjoy-binbin deleted the fix_eval_crash branch July 9, 2023 09:33
git-hulk pushed a commit to git-hulk/kvrocks that referenced this pull request Jul 30, 2023
The check allow we passing -1 is wrong, without
this fix, passing a -1 will crash the server.
@git-hulk git-hulk mentioned this pull request Jul 31, 2023
git-hulk pushed a commit that referenced this pull request Aug 1, 2023
The check allow we passing -1 is wrong, without
this fix, passing a -1 will crash the server.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-scripting area scripting A-security area security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants