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.

Sign up for GitHub

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

Jump to bottom

fix invalid read on corrupt ziplist #9831

Merged
merged 2 commits into from Nov 23, 2021
Merged

fix invalid read on corrupt ziplist #9831

merged 2 commits into from Nov 23, 2021

Conversation

oranagra
Copy link
Member

If the last bytes in ziplist are corrupt and we decode from tail to head, we may reach slightly outside the ziplist.

Found by ASAN on daily CI.
https://github.com/redis/redis/runs/4293567869?check_suite_focus=true

==23961==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6030000603ba at pc 0x0000005659a5 bp 0x7ffc328755f0 sp 0x7ffc328755e8
READ of size 1 at 0x6030000603ba thread T0
    #0 0x5659a4 in ziplistIndex /home/runner/work/redis/redis/src/ziplist.c:1163:13
    #1 0x4fb3a0 in quicklistPopCustom /home/runner/work/redis/redis/src/quicklist.c:1590:9
    #2 0x5ff4e6 in listTypePop /home/runner/work/redis/redis/src/t_list.c:66:13

0x6030000603ba is located 1 bytes to the right of 25-byte region [0x6030000603a0,0x6030000603b9)
allocated by thread T0 here:
    #0 0x4bfa1d in malloc (/home/runner/work/redis/redis/src/redis-server+0x4bfa1d)
    #1 0x54bd26 in ztrymalloc_usable /home/runner/work/redis/redis/src/zmalloc.c:108:17
    #2 0x54bd26 in ztrymalloc /home/runner/work/redis/redis/src/zmalloc.c:133:17
    #3 0x5da6a9 in rdbGenericLoadStringObject /home/runner/work/redis/redis/src/rdb.c:534:29

@oranagra oranagra added this to To Do in 6.2 Backport via automation Nov 23, 2021
@oranagra oranagra added this to Needs triage in Triage via automation Nov 23, 2021
@oranagra oranagra moved this from Needs triage to Awaits merge in Triage Nov 23, 2021
oranagra
oranagra commented Nov 23, 2021
View reviewed changes
tests/integration/corrupt-dump.tcl
start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] {
r debug set-skip-checksum-validation 1
r config set sanitize-dump-payload no
r restore _listbig 0 "\x12\x02\x02\x1B\x1B\x00\x00\x00\x16\x00\x00\x00\x05\x00\x00\x02\x5F\x39\x04\xF9\x02\x02\x5F\x37\x04\xF7\x02\x02\x5F\x35\xFF\x02\x19\x19\x00\x00\x00\x16\x00\x00\x00\x05\x00\x00\xF5\x02\x02\x5F\x33\x04\xF3\x02\x02\x5F\x31\xFE\xF1\xFF\x0A\x00\x64\x0C\xEB\x03\xDF\x36\x61\xCE"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sundb i guess that once the listpack PR is merged, RESTORE will fail and we'll need to catch it (graceful failure instead of an assertion)

sundb
sundb reviewed Nov 23, 2021
View reviewed changes
Copy link
Collaborator

@sundb sundb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

itamarhaber
itamarhaber approved these changes Nov 23, 2021
View reviewed changes
Copy link
Member

@itamarhaber itamarhaber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@oranagra oranagra merged commit a3a0142 into redis:unstable Nov 23, 2021
Triage automation moved this from Awaits merge to Done Nov 23, 2021
@oranagra oranagra deleted the ziplist_invalid_read branch November 23, 2021 12:56
sundb added a commit to sundb/redis that referenced this pull request Nov 24, 2021
@sundb
Change test added by redis#9831
b6764c1
hwware pushed a commit to hwware/redis that referenced this pull request Dec 20, 2021
@oranagra @hwware
fix invalid read on corrupt ziplist (redis#9831)
28b949b 
If the last bytes in ziplist are corrupt and we decode from tail to head,
we may reach slightly outside the ziplist.
@oranagra oranagra moved this from To Do to In progress in 6.2 Backport Apr 12, 2022
oranagra added a commit to oranagra/redis that referenced this pull request Apr 27, 2022
@oranagra
fix invalid read on corrupt ziplist (redis#9831)
95346cd 
If the last bytes in ziplist are corrupt and we decode from tail to head,
we may reach slightly outside the ziplist.

(cherry picked from commit a3a0142)
oranagra added a commit that referenced this pull request Apr 27, 2022
@oranagra
fix invalid read on corrupt ziplist (#9831)
e38d0b5 
If the last bytes in ziplist are corrupt and we decode from tail to head,
we may reach slightly outside the ziplist.

(cherry picked from commit a3a0142)
@oranagra oranagra moved this from In progress to Done in 6.2 Backport Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sundb sundb sundb left review comments

@itamarhaber itamarhaber itamarhaber approved these changes

@yossigo yossigo Awaiting requested review from yossigo

Assignees
No one assigned
Labels
None yet
Projects
Redis 6.2 Backport
Status: Done
6.2 Backport
Done
Triage
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@oranagra @sundb @itamarhaber