fix(asan): global-buffer-overflow in function escape_sds_argv of data_operations.cpp

[foreverneverer]

What problem does this PR solve?

Coredump

==12208== ERROR: AddressSanitizer: global-buffer-overflow on address 0x000000cf0a04 at pc 0x5fe0c5 bp 0x7fff4b80eee0 sp 0x7fff4b80eed8
READ of size 1 at 0x000000cf0a04 thread T0
    #0 0x5fe0c4 (/home/jiashuo1/work/pegasus/DSN_ROOT/bin/pegasus_shell/pegasus_shell+0x5fe0c4)
    #1 0x586894 (/home/jiashuo1/work/pegasus/DSN_ROOT/bin/pegasus_shell/pegasus_shell+0x586894)
    #2 0x59b230 (/home/jiashuo1/work/pegasus/DSN_ROOT/bin/pegasus_shell/pegasus_shell+0x59b230)
    #3 0x5abb16 (/home/jiashuo1/work/pegasus/DSN_ROOT/bin/pegasus_shell/pegasus_shell+0x5abb16)
    #4 0x462c28 (/home/jiashuo1/work/pegasus/DSN_ROOT/bin/pegasus_shell/pegasus_shell+0x462c28)
    #5 0x463156 (/home/jiashuo1/work/pegasus/DSN_ROOT/bin/pegasus_shell/pegasus_shell+0x463156)
    #6 0x465702 (/home/jiashuo1/work/pegasus/DSN_ROOT/bin/pegasus_shell/pegasus_shell+0x465702)
    #7 0x3639a1ecdc (/lib64/libc-2.12.so+0x1ecdc)
    #8 0x45fed8 (/home/jiashuo1/work/pegasus/DSN_ROOT/bin/pegasus_shell/pegasus_shell+0x45fed8)
0x000000cf0a04 is located 35 bytes to the right of global variable '*.LC37 (/home/jiashuo1/work/pegasus/src/shell/commands/data_operations.cpp)' (0xcf09e0) of size 1
  '*.LC37 (/home/jiashuo1/work/pegasus/src/shell/commands/data_operations.cpp)' is ascii string ''
0x000000cf0a04 is located 28 bytes to the left of global variable '*.LC38 (/home/jiashuo1/work/pegasus/src/shell/commands/data_operations.cpp)' (0xcf0a20) of size 4
  '*.LC38 (/home/jiashuo1/work/pegasus/src/shell/commands/data_operations.cpp)' is ascii string '
11'
Shadow bytes around the buggy address:
  0x0000801960f0: 00 00 00 00 00 00 00 00 00 00 00 00 f9 f9 f9 f9
  0x000080196100: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080196110: 00 00 00 00 00 07 f9 f9 f9 f9 f9 f9 00 05 f9 f9
  0x000080196120: f9 f9 f9 f9 00 00 00 00 00 00 00 00 07 f9 f9 f9
  0x000080196130: f9 f9 f9 f9 00 00 04 f9 f9 f9 f9 f9 01 f9 f9 f9
=>0x000080196140:[f9]f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
  0x000080196150: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
  0x000080196160: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
  0x000080196170: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080196180: 06 f9 f9 f9 f9 f9 f9 f9 03 f9 f9 f9 f9 f9 f9 f9
  0x000080196190: 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:     fa
  Heap righ redzone:     fb
  Freed Heap region:     fd
  Stack left redzone:    f1
  Stack mid redzone:     f2
  Stack right redzone:   f3
  Stack partial redzone: f4
  Stack after return:    f5
  Stack use after scope: f8
  Global redzone:        f9
  Global init order:     f6
  Poisoned by user:      f7
  ASan internal:         fe
==12208== ABORTING

Related Code

https://github.com/XiaoMi/pegasus/blob/51f868f031fed9b8fbff7aa0d0e601176944523b/src/shell/commands/data_operations.cpp#L2429-L2438

Reason

The problem at line 2433：
https://github.com/XiaoMi/pegasus/blob/51f868f031fed9b8fbff7aa0d0e601176944523b/src/shell/commands/data_operations.cpp#L2433
When pass “”(empty string), sdsnewlen will excute the code:
https://github.com/XiaoMi/pegasus/blob/51f868f031fed9b8fbff7aa0d0e601176944523b/src/shell/sds/sds.c#L134-L136
because the initlen > length of init(also is ""), it will core.

The problem will happen on all functions which use the escape_sds_argv

What is changed and how it works?

Actually, it has two resolutions:

• one way, also this pr is pass one blank string which length equal with initlen（also the dest_len）
• another way, it can pass NULL to skip memcpy(s, init, initlen) and directly execte s[initlen] = '\0'

I have test the two resolutions and it work well, but I'm not sure that pass NULL (resolution 2) whether to cause other problem.

Check List

• Manual test (add detailed scripts or steps below)

Related changes

• Need to cherry-pick to the release branch
• Need to be included in the release note

[vagetablechicken]

sdsnewlen is a func in redis, maybe you can checkout if it has been resolved in redis.

[vagetablechicken]

About passing NULL for 'init', it has a rule
https://github.com/XiaoMi/pegasus/blob/51f868f031fed9b8fbff7aa0d0e601176944523b/src/shell/sds/sds.c#L74

[foreverneverer]

About passing NULL for 'init', it has a rule
https://github.com/XiaoMi/pegasus/blob/51f868f031fed9b8fbff7aa0d0e601176944523b/src/shell/sds/sds.c#L74

Yeah, I know, so I test NULL

[foreverneverer]

sdsnewlen is a func in redis, maybe you can checkout if it has been resolved in redis.

No, I think it is that we use it mistake.

[vagetablechicken]

About passing NULL for 'init', it has a rule
https://github.com/XiaoMi/pegasus/blob/51f868f031fed9b8fbff7aa0d0e601176944523b/src/shell/sds/sds.c#L74

Yeah, I know, so I test NULL

So You don't have to worry about resolution 2(”NULL“) whether to cause other problem.

[foreverneverer]

I have test the two resolutions and it work well, but I'm not sure that pass NULL (resolution 2) whether to cause other problem.

in latest commit, I choose resolution 2