An update on 0day CVE-2021-43798: Grafana directory traversal (opens in new tab)

(grafana.com)

95 pointsep_jhu4y ago23 comments

23 comments

Good ol path traversal https://github.com/grafana/grafana/commit/c798c0e958d15d9cc7...

This part is kind of interesting — it looks like gosec at least gave the opportunity to catch this but that was missed due a misunderstanding of what was in scope for filepath.Clean:

https://github.com/grafana/grafana/commit/c798c0e958d15d9cc7...

TheDong4y ago

I mean, can you blame them?

The docs for filepath.Clean state the following:

    ...
    3. Eliminate each inner .. path name element (the parent directory)
       along with the non-.. element that precedes it.
    4. Eliminate .. elements that begin a rooted path:
       that is, replace "/.." by "/" at the beginning of a path,
       assuming Separator is '/'.

There is no mention that this function cannot be used for security, or that it does not safely ensure a path can't refer to a parent directory.

A quick reading of 3 and 4 will make you assume that a path has no ".."s after being "Clean"d. If you actually think about it more, you'll realize that of course it will leave ".." at the beginning of relative paths, but the docs do not make it clear, and I can understand why a programmer might reach for this.

There's no clearer function to reach for either, like "filepath.DirectoryContains(parent, path) bool".

3 more replies

RichiH4y ago

Important note: I mixed up CVE-2021-41090 and CVE-2021-43798 in the initial version of the blog post. While that has been corrected and a note added to the blog post, it still lead to some confusion.

The 0day is only for Grafana-the-software, not for the Grafana Agent.

Also important to note: While the overall course of events is clearly less than ideal, we still strongly believe that Jordy did us good. Mistakes happen, and the intention was good. Overall, Grafana is now more secure than it was last week.

shiftyck4y ago

I wrote a script today to try and exploit this on our Grafana 8.1.2 instance but couldn't. Using Oauth for auth and only got 302 redirects back to the login page. Anyone else able to exploit this with Oauth?

vasuki4y ago

It might be because of path normalization by your http client. For example, with `curl` you will also need to use `--path-as-is` to correctly test traversal. Another reason could be path normalization by the reverse proxy/WAF.

> --path-as-is

> Tell curl to not handle sequences of /../ or /./ in the given URL path. Normally curl will squash or merge

> them according to standards but with

>this option set you tell it not to do that.

> Added in 7.42.0.

WoahNoun4y ago

> 2021-12-03 08:42: Jordy tweets and deletes about “read arbitrary files on the host, no authentication needed” (Editor’s note: We were not aware of this until 2021-12-07.)

Doesn't quite sound like an "ethical hacker" to me.

RichiH4y ago

I talked to Jordy about it. It was his first CVSS HIGH vulnerability and he was super happy & excited about it. While we would have preferred if things went differently, it was an honest mistake.

On balance, I still prefer if someone approaches us with good intentions and messes up a bit over someone simply dumping a 0day into the wild or into private circles. And this way, we at least had a tested patch in hand already and knew that Grafana Cloud was not affected.

And as per https://news.ycombinator.com/item?id=29495431 ... we all make mistakes.

tomsioncati4y ago

Echoing this. It was a good find and well intended. We'll welcome any future submissions from Jordy.

ysleepy4y ago

As far as I can see the post does not mention the affected releases nor the versions to upgrade to.

Is 8.3.1 patched?

bboreham4y ago

Yes. It's in the advisory: https://grafana.com/blog/2021/12/07/grafana-8.3.1-8.2.7-8.1....

404mm4y ago

Affects all 8.x releases

remram4y ago

I just realized I'm still on 7.3 because I could never get the image renderer to work on 8.x (the thing that shows the graph in the alert email). Anyone got a Docker image that will render images out of the box?

[edit: Just installed 8.3. Alert conditions warn they are now "beta feature" and "could stop working in next version". Is Grafana removing features every release?]

Beltiras4y ago

Note: postmortem has a more dire meaning in non-tech circles (literally means "after death"). You want to say retrospective instead. I know it's a difference in culture.

remram4y ago

I think people used to publish "postmortems" for projects that were literally over, either because they failed or because they were published long ago. At some point people started using it for outages, then security issues too.

I agree that the term is inadequate, I can just see how we got here.

graffgejrkk4y ago

> 2021-12-03: Release plan set: 2021-12-07 for private customer release, 2021-12-14 for public release

Does someone know why they were playing on sitting on the public release for a week after private release?

Seems that by doing this they allowed it to become a 0day.

alphahsl4y ago

> Customers get a week to upgrade under strict embargo

Seems that they can enforce an embargo with their private customers so that it won't become a 0day.

It became a 0day because the security researcher inadvertently kicked off public discussion.

alphahsl4y ago

I should clarify that “inadvertently” is the key word here. The 0day happened because of a number of factors detailed in the post, and it isn’t the fault of any one person.

1 more reply

j / k navigate · click thread line to collapse

23 comments

nerdbaggy4y ago

Good ol path traversal https://github.com/grafana/grafana/commit/c798c0e958d15d9cc7...

acdha4y ago

This part is kind of interesting — it looks like gosec at least gave the opportunity to catch this but that was missed due a misunderstanding of what was in scope for filepath.Clean:

https://github.com/grafana/grafana/commit/c798c0e958d15d9cc7...

TheDong4y ago

I mean, can you blame them?

The docs for filepath.Clean state the following:

    ...
    3. Eliminate each inner .. path name element (the parent directory)
       along with the non-.. element that precedes it.
    4. Eliminate .. elements that begin a rooted path:
       that is, replace "/.." by "/" at the beginning of a path,
       assuming Separator is '/'.

There is no mention that this function cannot be used for security, or that it does not safely ensure a path can't refer to a parent directory.

There's no clearer function to reach for either, like "filepath.DirectoryContains(parent, path) bool".

3 more replies

RichiH4y ago

Important note: I mixed up CVE-2021-41090 and CVE-2021-43798 in the initial version of the blog post. While that has been corrected and a note added to the blog post, it still lead to some confusion.

The 0day is only for Grafana-the-software, not for the Grafana Agent.

shiftyck4y ago

vasuki4y ago

> --path-as-is

> Tell curl to not handle sequences of /../ or /./ in the given URL path. Normally curl will squash or merge

> them according to standards but with

>this option set you tell it not to do that.

> Added in 7.42.0.

WoahNoun4y ago

> 2021-12-03 08:42: Jordy tweets and deletes about “read arbitrary files on the host, no authentication needed” (Editor’s note: We were not aware of this until 2021-12-07.)

Doesn't quite sound like an "ethical hacker" to me.

RichiH4y ago

I talked to Jordy about it. It was his first CVSS HIGH vulnerability and he was super happy & excited about it. While we would have preferred if things went differently, it was an honest mistake.

And as per https://news.ycombinator.com/item?id=29495431 ... we all make mistakes.

tomsioncati4y ago

Echoing this. It was a good find and well intended. We'll welcome any future submissions from Jordy.

ysleepy4y ago

As far as I can see the post does not mention the affected releases nor the versions to upgrade to.

Is 8.3.1 patched?

bboreham4y ago

Yes. It's in the advisory: https://grafana.com/blog/2021/12/07/grafana-8.3.1-8.2.7-8.1....

404mm4y ago

Affects all 8.x releases

remram4y ago

[edit: Just installed 8.3. Alert conditions warn they are now "beta feature" and "could stop working in next version". Is Grafana removing features every release?]

Beltiras4y ago

Note: postmortem has a more dire meaning in non-tech circles (literally means "after death"). You want to say retrospective instead. I know it's a difference in culture.

remram4y ago

I agree that the term is inadequate, I can just see how we got here.

graffgejrkk4y ago

> 2021-12-03: Release plan set: 2021-12-07 for private customer release, 2021-12-14 for public release

Does someone know why they were playing on sitting on the public release for a week after private release?

Seems that by doing this they allowed it to become a 0day.

alphahsl4y ago

> Customers get a week to upgrade under strict embargo

Seems that they can enforce an embargo with their private customers so that it won't become a 0day.

It became a 0day because the security researcher inadvertently kicked off public discussion.

alphahsl4y ago

I should clarify that “inadvertently” is the key word here. The 0day happened because of a number of factors detailed in the post, and it isn’t the fault of any one person.

1 more reply

j / k navigate · click thread line to collapse