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

Null pointer exception while parsing package-lock.json #5318

Closed
ferben opened this issue Jan 16, 2023 · 5 comments · Fixed by #5339
Closed

Null pointer exception while parsing package-lock.json #5318

ferben opened this issue Jan 16, 2023 · 5 comments · Fixed by #5339

Comments

@ferben
Copy link

ferben commented Jan 16, 2023

Suspiciously similar as #1603

Dependency-Check Core version 8.0.0

output:

[WARN] An unexpected error occurred during analysis of 'C:\...\NodeJS_64\package-lock.json' (Node.js Package Analyzer): null
[ERROR]
java.lang.NullPointerException: null
        at org.glassfish.json.JsonObjectBuilderImpl$JsonObjectImpl.getString(JsonObjectBuilderImpl.java:257)
        at org.owasp.dependencycheck.analyzer.NodePackageAnalyzer.processDependencies(NodePackageAnalyzer.java:397)
        at org.owasp.dependencycheck.analyzer.NodePackageAnalyzer.analyzeDependency(NodePackageAnalyzer.java:270)
        at org.owasp.dependencycheck.analyzer.AbstractAnalyzer.analyze(AbstractAnalyzer.java:131)
        at org.owasp.dependencycheck.AnalysisTask.call(AnalysisTask.java:88)
        at org.owasp.dependencycheck.AnalysisTask.call(AnalysisTask.java:37)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
        at java.base/java.lang.Thread.run(Thread.java:829)

package-lock.json.zip

@rturner-edjuster
Copy link
Contributor

Possibly related to #1947 and #4293 ?

@rturner-edjuster
Copy link
Contributor

Not sure if this is the correct approach, or if it may cause other "issues", but it should avoid the crashing, but may pass handling of empty versions to later code which may not handle the situation properly. I did a quick scan of immediately surrounding code, and it looks like it would be okay, but that doesn't mean it will work properly:

diff --git a/core/src/main/java/org/owasp/dependencycheck/analyzer/NodePackageAnalyzer.java b/core/src/main/java/org/owasp/dependencycheck/analyzer/NodePackageAnalyzer.java
index 12875f915..28976bd14 100644
--- a/core/src/main/java/org/owasp/dependencycheck/analyzer/NodePackageAnalyzer.java
+++ b/core/src/main/java/org/owasp/dependencycheck/analyzer/NodePackageAnalyzer.java
@@ -394,7 +394,7 @@ public class NodePackageAnalyzer extends AbstractNpmAnalyzer {
                         continue;
                     }

-                    version = jo.getString("version");
+                    version = jo.getString("version", "");
                     optional = jo.getBoolean("optional", false);
                     isDev = jo.getBoolean("dev", false);
                 } else {

@uwesinha
Copy link

uwesinha commented Jan 16, 2023

I did some "printf debugging" (or rather "LOGGER.info() debugging" 😉) this afternoon, and I think I found the source of the problem:

The "packages" element in our package-lock.json has a child element with the empty string as its name and relevant portions of package.json as its value.

Click for a package-lock.json example
{
  "name": "my-fancy-project",
  "lockfileVersion": 2,
  "requires": true,
  "packages": {
    "": {
      "name": "my-fancy-project",
      "license": "WHATEVER",
      "dependencies": {
        "some-dependency": "3.0.0",
        "another-dep": "5.2.0",
        [...truncated for brevity...]
      },
      "devDependencies": {
        "@babel/core": "7.20.5",
        "babel-loader": "9.1.0",
        [...truncated for brevity...]
      },
      "engines": {
        "node": "18.12.1",
        "npm": "8.19.2"
      }
    },
    "node_modules/@babel/core": {
      "version": "7.20.5",
      "resolved": "https://registry.npmjs.org/@babel/core/-/core-7.20.5.tgz",
      "integrity": "sha512-UdOWmk4pNWTm/4DlPUl/Pt4Gz4rcEMb7CY0Y3eJl5Yz1vI8ZJGmHWaVE55LoxRjdpx0z259GE9U5STA9atUinQ==",
      "dev": true,
      "dependencies": {
        [...truncated for brevity...]
      },
      "engines": {
        "node": ">=6.9.0"
      },
      "funding": {
        "type": "opencollective",
        "url": "https://opencollective.com/babel"
      }
    },
    [...more actual dependencies follow...]
  }
}

Edit: This can also be seen in the package-lock.json which @ferben provided along with his comment.

According to the NPM docs, this is the intended behavior, starting with NPM v7:

The root project is typically listed with a key of "", and all other packages are listed with their relative paths from the root project folder.

The NodePackageAnalyzer tries to analyze this element anyway. But since the value-part does not have child element named "version", the method call jo.getString("version") in line 397 causes the NullPointerException we all hate.

jeremylong added a commit that referenced this issue Jan 18, 2023
resolves #5318
@jeremylong jeremylong mentioned this issue Jan 18, 2023
@uwesinha
Copy link

With 8.0.1 the NPE is gone at last. 👍

@lmo-wt lmo-wt mentioned this issue Jan 26, 2023
@lmo-wt
Copy link
Contributor

lmo-wt commented Jan 26, 2023

I am still encountering the issue from the NpmPayloadBuilder class.
I opened a PR that fixes the issue.
#5390

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

Successfully merging a pull request may close this issue.

4 participants