Skip to content

fix redis: check geo reply member is a string before GetString#1288

Open
netliomax25-code wants to merge 1 commit into
userver-framework:developfrom
netliomax25-code:redis-geo-reply-type-check
Open

fix redis: check geo reply member is a string before GetString#1288
netliomax25-code wants to merge 1 commit into
userver-framework:developfrom
netliomax25-code:redis-geo-reply-type-check

Conversation

@netliomax25-code

Copy link
Copy Markdown
Contributor

ParseReplyDataArray for GeoPoint in redis/src/storages/redis/parse_reply.cpp reads the first element of each per-member sub-array of a GEORADIUS/GEOSEARCH reply with GetString(), but only checks that the sub-array is non-empty rather than that the element is a string. ReplyData::GetString() resolves through std::get_if, which returns nullptr for a non-string element, so a compromised or MITM'd Redis server that returns an integer or nested array there makes the parser dereference that null pointer while move-constructing member (a UASSERT abort under the sanitizer CI, a null-pointer dereference of a std::string in release). I added an IsString check before the read that throws ParseReplyException, matching the type checks the sibling branches already perform, and covered both the rejected and accepted reply shapes with a test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant