draios / sysdig

Linux system exploration and troubleshooting tool with first class support for containers
http://www.sysdig.com/
Other
7.74k stars 729 forks source link

Linking against jq exposes attack surface #643

Closed hlieberman closed 1 year ago

hlieberman commented 8 years ago

Hello sysdig folks!

As I expressed on Twitter, I'm a bit concerned about the state of jq. As awesome as it is, it seems to be going through somewhat of a low-point in its maintenance. Considering sysdig's hooks into the kernel, I want to make sure that none of the problems extant in jq could cause more severe problems when used by Sysdig, especially on untrusted input. (A situation where sysdig is frequently used is on systems which might be compromised.)

Specifically, CVE-2015-8863 and CVE-2016-4074 aren't fixed in any released version of jq. There hasn't been any code changes since January, and no release since August '15.

I'm not saying that linking against jq isn't safe --- just that I'd like more investigation to check before we push it. (I'm going to be opening a bug in Debian so that we /can/ link against jq, as currently we don't ship a -dev version with the code.)

faxm0dem commented 8 years ago

:+1:

aleks-f commented 8 years ago

Hello Harlan,

Thanks for reporting and opening the issue. Given that CVE-2015-8863 is fixed in the jq development branch and for CVE-2016-4074 a reasonable workaround is proposed, would bundling up a patched version of released jq with sysdig be a viable option for Debian package?

Currently, the code dependent on jq is actually not yet directly used by sysdig, but we were just about to introduce that dependency and the plan for it is to only be used when dealing with JSONs from known sources, such as eg. Kubernetes or Mesos/Marathon API servers, which are considered reliable sources. There are no plans to ingest JSON from unknown sources and, more importantly, the kernel module does not and will not use jq at all.

In any case, we definitely want to hear on the progress of your investigation and potential proposals on how to deal with this issue, so please keep us updated and let us know your suggestions.

hlieberman commented 8 years ago

I can talk to the Debian maintainer of jq about taking the fix for CVE-2015-8863 out of cycle, which would solve that issue. The workaround for CVE-2016-4074 isn't great; the right way to do it is something like https://github.com/stedolan/jq/issues/1136#issuecomment-217185232 where you do a depth check of the tree.

I'm not 100% sure that I'd call the Kubernetes or Mesos servers fully trusted, but I could certainly be convinced of it. I was more worried about the potential of sysdig interpreting, say, JSON passing over the wire.

I'll reach out to the Debian jq maintainer, see what they think.

aleks-f commented 8 years ago

Yes, tree depth check is what I was referring to - it seems like a relatively easy fix.

As for trusted servers, of course, anything can be compromised if people are careless. What I meant is that typically these API servers are not opened to public internet access but rather serve content internally, in an intranet or VPN setting, which makes them less exposed to attacks; we expect/handle very specific things from them in terms of JSON structure, so an exploit would have to be within those boundaries, which makes it much harder and, I'd dare say, impossible for the tree depth bug.

In any case, thanks for the quick response and keep us updated.

hlieberman commented 8 years ago

Filed Debian Bug #833213 to track the -dev issue, commented on the preexisting #802231 about the CVE-2015 fix.

hlieberman commented 7 years ago

Hello @aleks-f. Unfortunately, it seems the Debian maintainer of jq is also inactive, and there's been no further progress in getting any changes made upstream.

At this moment, I can't build a new version of sysdig for Debian, and the freeze for Debian Stretch is rapidly approaching. I'm not sure how deep the integration is, but if there's only a light link between sysdig and jq, or if it's easy to substitute, I suggest dropping jq as a dependency.

aleks-f commented 7 years ago

Thanks for the heads up @hlieberman.

Few questions/proposals:

1) If we provide patches for said jq vulnerabilities, that would take care of the problem, right? I can see there are other offers along this line.

2) I can not find bug 8333213; can you point to the right place?

3) Is the freeze date Jan 7 2017 ?

Thanks

hlieberman commented 7 years ago

On November 14, 2016 11:12:39 AM EST, Aleksandar Fabijanic notifications@github.com wrote:

Thanks for the heads up @hlieberman.

Of course!

Few questions/proposals:

1) If we provide patches for said jq vulnerabilities, that would take care of the problem, right? I can see there are other offers along this line.

I've actually filed an NMU to patch the security vulnerabilities. If I don't hear back from the Debian maintainer, it should get automatically fixed in the next few days.

2) I can not find bug 8333213; can you point to the right place?

Sorry, too many threes. Should be 833213.

3) Is the freeze date Jan 7 2017 ?

Correct!

Harlan Lieberman-Berg ~hlieberman

aleks-f commented 7 years ago

Great, thanks.

Just to make sure I understood correctly: you do not need anything from us in regards with this issue and everything will be taken care of for the next Debian release?

hlieberman commented 7 years ago

Sort of! You don't need anything for the next Debian release, but I'd like you to keep an eye open for alternatives to jq out there. The upstream is still dead, and though the security issues so far are patched, that's not necessarily the end of them forever. Food for thought!

nh2 commented 7 years ago

I find it questionable to rely on specific operating system distributions to distribute security relevant patches that upstream doesn't (seem to) care to release in stable versions themselves.

I've filed https://github.com/stedolan/jq/issues/1406 to clarify the issue.

nh2 commented 7 years ago

Why did my comment unnassign @aleks-f from this issue?

aleks-f commented 7 years ago

Because I am not member of sysdig team anymore. Check with @luca3m about the future of this issue - last I've heard, jq dependency will go away.

nh2 commented 7 years ago

Because I am not member of sysdig team anymore

Ah, I got confused then, I thought it was my comment that automatically did it, but you simply did it yourself right after I posted.

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.