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

Please clarify fixing commits for CVE-2019-10193 #6214

Closed
lamby opened this issue Jul 8, 2019 · 12 comments
Closed

Please clarify fixing commits for CVE-2019-10193 #6214

lamby opened this issue Jul 8, 2019 · 12 comments

Comments

@lamby
Copy link
Contributor

lamby commented Jul 8, 2019

Recently, two CVEs were filed for redis:

For the first (ie. CVE-2019-10192) it is clear that this was fixed in 9f13b2b and e216cea. However, for the second (ie. CVE-2019-10193), it is not clear what commits will remedy this.

I've done a quick glance at the git blame output for this function with no dice...

Any pointers? :)

@antirez
Copy link
Contributor

antirez commented Jul 8, 2019

Hi @lamby, the CVEs are mostly unreadable. The HyperLogLog issues where fixed in the following commits.

  • e216cea HyperLogLog: handle wrong offset in the base case.
  • a4b90be HyperLogLog: enlarge reghisto variable for safety.
  • 9f13b2b Fix hyperloglog corruption

The other vulnerability was a DoS that was fixed here: 5b52bc7

The above CVEs have no description of what is going on at all: just in one there is a link to bugzilla with little more info, so well... not sure what to say.

@antirez
Copy link
Contributor

antirez commented Jul 8, 2019

P.S. I think I'll release a security advisor myself as a Redis issue.

@lamby
Copy link
Contributor Author

lamby commented Jul 8, 2019

e216cea HyperLogLog: handle wrong offset in the base case.
9f13b2b Fix hyperloglog corruption

Those appear to be part of CVE-2019-10192. Are you saying that:

a4b90be HyperLogLog: enlarge reghisto variable for safety.

… is also part of that, or are you completely in the dark too? ^_^

The other vulnerability was a DoS that was fixed here: 5b52bc7

This doesn't sound like CVE-2019-101923 from its description, so maybe there's another CVE number waiting to be assigned.

Hm. :)

@antirez
Copy link
Contributor

antirez commented Jul 8, 2019

@lamby basically a4b90be is another bug that I found after I got the report for the other hyperloglog bug. It's a similar issue I found doing the auditing. @JohnSully reported an out of range memory access, I found another similar in another place, and also I found another bug again related to hyperloglog and registers access in yet another place. That's the story.

@lamby
Copy link
Contributor Author

lamby commented Jul 8, 2019

Cheer for sharing. Okay, I'll give it a few days or so for that Bugzilla link to throw up something interesting about CVE-2019-101923 ...

@antirez
Copy link
Contributor

antirez commented Jul 8, 2019

I released a statement here @lamby #6215

@antirez antirez closed this as completed Jul 8, 2019
@lamby
Copy link
Contributor Author

lamby commented Jul 9, 2019

Whilst I thank you releasing that advisory statement, I don't think we have resolved the question of CVE-2019-101923 but I grant that is not in your hands. Lets move all discussion to #6215.

@JohnSully
Copy link
Contributor

I found the original issues. Feel free to contact me if you have any questions. john at csquare dot ca.

@lamby
Copy link
Contributor Author

lamby commented Jul 9, 2019

@JohnSully Can you share here? I'm, in particular, interested in CVE-2019-101923

@JohnSully
Copy link
Contributor

I thought Antirez explained it well, but that is probably because I'm too close to the actual code. The short story is Redis was computing addresses relative to the stack pointer that could be directly influenced by user data. As part of a hyperloglog datastructure there are sparse areas, and when Redis tries to convert them to non-sparse it skips over them in the array allocated on the stack. Because it failed to bounds check you could overflow. With some limitations that overflow and the data written was controlled by the attacker.

I'm not a security researcher so I'm not quite used to heavily distilling these bugs. Let me know if you need more/different details. My main interest is I run a fork called KeyDB and was performing security testing on that code.

@lamby
Copy link
Contributor Author

lamby commented Jul 9, 2019

Thanks. However, whilst I understood how/what the referenced commits are fixing I am trying to — from a closing all loopholes / administrative point of view — working what, exactly, that latter CVE is referring to. If it is closed by one of the commits, that's great... but that is still not clear. :)

@JohnSully
Copy link
Contributor

Ah sorry for the misunderstanding. Since the commits are Antirez's I'll let him answer.

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

No branches or pull requests

3 participants