Security vulnerabilities with spf13/cobra dependency

[Polber]

Problem

Currently, containerd/continuity directly imports the spf13/cobra v1.0.0 package in the go.mod file. The spf13/cobra v1.0.0 package imports spf13/viper v1.4.0 which imports coreos/etcd v3.3.10+incompatible. This is a problem because coreos/etcd v3.3.10+incompatible has a known security vulnerability with authentication when RBAC is used and client-cert-auth is enabled.

More information about the coreos/etcd security issue can be found here: https://nvd.nist.gov/vuln/detail/CVE-2018-16886

spf13/viper v1.4.0 also imports gorilla/websocket v1.4.0 which has a security vulnerability that allows for a potential DoS attack.

More information about the gorilla/websocket security issue can be found here: https://nvd.nist.gov/vuln/detail/CVE-2020-27813

Fix

1. The easiest fix is to upgrade the dependency on spf13/cobra to v1.4.0 as this version removes the dependency on viper entirely, and therefore removes the dependency on coreos/etcd and gorilla/websocket.
2. If it is not possible to upgrade spf13/cobra to v1.4.0, any version of spf13/cobra v1.2.0-v1.3.0 will import versions of coreos/etcd and gorilla/websocket that are safe to use, as of now.

Update

It appears as though PR #194 bumps spf13/cobra to v1.3.0 which would solve this issue should it get merged.

[AkihiroSuda]

Seems a false-positive.
I don’t see etcd and gorilla/websocket in https://github.com/containerd/continuity/blob/main/vendor/modules.txt

[Polber]

@AkihiroSuda Those packages are not directly imported by containerd/continuity, however, they are sub-dependencies imported by spf13/cobra which I explained in more detail in my initial comment. Upgrading spf13/cobra to 1.2.0 or greater will cause the correct versions of the sub-dependencies to be imported. If you check the go.sum file which lists sub-dependencies, you will notice all the aforementioned packages are there. I would like to kindly ask you keep this issue open and marked as valid as there is validity to the security threat.

[AkihiroSuda]

Disagree. The vulnerable packages are not actually used.
https://github.com/containerd/continuity/tree/main/vendor

[dmcgowan]

@Polber the security issue is not relevant to this project directly, as Akihiro pointed out. I want to better understand the indirect issue though. If a package were to import both continuity and make use of the packages which had the vulnerability, are you saying go.mod would end up choosing the indirect version from continuity rather than the explicit version in that importing library?

[Polber]

@AkihiroSuda I now see your point. I'm not sure it is best practice to support packages with known vulnerabilities, whether they are used or not, but I suppose if it can be confirmed that the exploits cannot be triggered within this package, then that will suffice for now. However, upgrading spf13/cobra to v1.4.0 removes the path to the insecure packages altogether, which could help remove the possibility of future vulnerabilities being found

[Polber]

@dmcgowan I don't think that is the case. The case I was worried about is if continuity is using a function in cobra, that uses a function in viper, that uses one of the affected packages, then there would be a threat to continuity. However, if you both are telling me that is not the case, then I suppose there isn't a problem, for now.

The indirect sub-dependencies get imported because the direct dependencies in continuity require them, but that does not affect what packages/versions get imported by another package other than continuity. That would be determined by that package's go.mod file

[estesp]

this can be closed now that #200 is merged and there is no more dependency on cobra in the continuity library.