lein-nvd's own dependencies shouldn't affect the analysis result

[vemv]

Hi,

first of all thanks again for this tool!

There's a thing that affects its cleanliness: the dependencies of the lein-nvd plugin itself affect the workload that is passed to the NVD analyzer.

A notorious example being guava, which is not necessarily present in my apps, and yet can trigger a vuln report, because as of today lein-nvd depends on (or resolves to) an outdated Guava version.

That was no clean fix AFAIK - none of these are ideal:

;; Approach 1 - global dependency.
;; Problematic: introduces a false dependency (the givenapp doesnt use Guava in any way)
:dependencies [[com.google.guava/guava "25.1-jre"]]

;; Approach 2 - :provided dependency.
;; Problematic: also introduces a false dependency, just in a weaker form
:provided {:dependencies [[com.google.guava/guava "25.1-jre"]]}

;; Approach 3 - NVD-specific dependency, run lein-nvd later with `+nvd` profile
;; Problematic: if our app actually depends on Guava at a global level
;; this dependency will override that version, mutating the analysis results.
:nvd {:dependencies [[com.google.guava/guava "25.1-jre"]]}

I would suggest either:

• figuring out a way of setting up Lein deps optimally (documenting that in the README); or
• run lein-nvd in two phases somehow
  • first one without nvd or any of its deps
  • second one with nvd, analyzing the classpath of the previous phase

WDYT?

Thanks - V

[vemv]

Hi @rm-hull - have you had the chance to give this one a think?

[raymcdermott]

This is hurting us too

[sogaiu]

I don't suppose https://github.com/benedekfazekas/mranderson could help somehow...

[vemv]

Taking a look at https://github.com/rm-hull/lein-nvd/blob/master/src/nvd/task/check.clj, I think that it's pretty easy to hack a solution: main could take a vanilla, discrete classpath string as stdin. Continue operation from there.

This would be offered as an additional -main-like function, for not breaking anything.

That also would simplify both the Lein and deps.edn integration - right now classpaths are kind of second-guessed instead of being taken as-is. Personally I think that a simpler, unmistakable approach would be benefitial for a security tool.

Even if it means spawning 2 JVMs for full operation (which is exactly what happens when one does lein repl anyway)

I don't use lein-nvd atm (although I'd love to) and I also can't contribute to OSS atm. But I think such a PR would be quite simple to accomplish and most likely be accepted (as it can be a merely accretive change).

[dotemacs]

That also would simplify both the Lein and deps.edn integration - right now classpaths are kind of second-guessed instead of being taken as-is.

This intrigues me, how would you obtain this classpath? What's your thinking? Thanks

[vemv]

lein classpath (which of course can be refined with with-profile) outputs a single discrete string

[dotemacs]

lein classpath (which of course can be refined with with-profile) outputs a single discrete string

OK, cool. I was thinking that you might be considering a library of some sort.

I guess for Clojure CLI, the equivalent would be clj -Spath.

[dotemacs]

The way you used "unmistakable approach" and "discrete", put me on a back foot and I wondered what am I missing.
I actually had to look them up in a dictionary :)

Looking at the output of: $ clj -Spath and

(require '[clojure.java.classpath :as cp])
(map (fn [jar] (.getPath jar)) (cp/classpath))

you get the same output. The same jars are listed.

What I'm trying to say here is that classpath of a library/app can be obtained programmatically, you don't have to "shell out" via lein classpath or clj -Spath. The only thing that you'd have to handle is the profiles (in Leiningen) and aliases (in Clojure CLI deps.edn). Once you have that, if you get the jars of this library and remove them from the list, this issue can be resolved.

Unless I'm missing something?

[vemv]

you get the same output. The same jars are listed.

That doesn't falsify the fact the lein-nvd is making a best-effort approach to figuring out what the classpath should be. The result will generally be the same, but there's no guarantee of that, as Lein, tools-deps, the JDK, etc are all subject to change.

We also consider that our own tests (e.g. unit tests, or experimentation in the repl) are not guaranteed to be exhaustive.

Unless I'm missing something?

For removing lein-nvd and its transitive dependencies from a current classpath, while also not removing any overlapping dependencies seems a non-trivial effort.

While something like the following:

lein with-profile -nvd classpath | lein with-profile +nvd run -m nvd.thing-i-am-proposing

would perform no classpath reconstruction, and no removals from any classpath.

[raymcdermott]

this is a terrible hack but it's what this bug is forcing me. and I assume others, to work on

## Scan the dependencies with nvd
clojure -M:nvd 2> /dev/null | grep CVE > cves

cat cves

deps=$(cat cves | grep CVE | awk '{print $2}' | sed -e 's/-[[:digit:]]*\.[[:digit:]]*\.[[:digit:]]*.jar.*//' -e 's/.jar//')

## Obtain the dependencies without the nvd deps
clojure -Stree > standard-deps

found_cves=0

for dep in $deps
do
	grep $dep standard-deps
	if [ $? -eq 0 ]
	then
		echo ">> $dep FOUND in standard path."
		found_cves=1
	fi
done

if [ ${found_cves} -eq 0 ] 
then
	echo "No CVEs from project dependencies - only from NVD dependencies"
else
	echo "CVEs found in project dependencies. Check reports in target/nvd"
fi


rm cves standard-deps

I will be very happy to remove this script.