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

The package accepts NULL values in arrays whose types do not expect NULL #231

Closed
paulshryock opened this issue Mar 15, 2024 · 4 comments
Closed

Comments

@paulshryock
Copy link

Summary

A Checkmarx SCA scan has identified a "High" severity vulnerability in this package.

Screenshot of Checkmarx scan showing vulnerability

About

The package netresearch/jsonmapper accepts NULL values in arrays whose types do not expect NULL. This flaw allows an attacker to crash the server by sending malformed JWT JSON in "LoginPacket". This vulnerability also affects pocketmine/pocketmine-mp package versions prior to 4.23.1, and 5.x prior to 5.3.1 due to dependency of netresearch/jsonmapper.

Category: CWE-400

The software does not properly control the allocation and maintenance of a limited resource, thereby enabling an actor to influence the amount of resources consumed, eventually leading to the exhaustion of available resources.

References

@SvenRtbg
Copy link
Contributor

The proposed fix only fixes one specific application that enables a setting. Not checking for null values or not validating that the JSON sent conforms to a specific schema is the task of the software itself.

Please show code that actually "crashes the server due to malformed JWT JSON in LoginPacket" for this library.

This is the second time today that an arbitrary Security Scanner report has been handed in without offering detailed information beyond "it is seriously bad". Without additional explaination and possibly demonstration of the problem, I might come to the conclusion this is someone being on a mission to attract attention, or distract from a topic not obvious.

@cweiske
Copy link
Owner

cweiske commented Mar 16, 2024

Duplicate of #230.

@cweiske cweiske closed this as completed Mar 16, 2024
@paulshryock
Copy link
Author

paulshryock commented Mar 18, 2024

@SvenRtbg I hear you. Unfortunately, this is all the context I was given in the Checkmarx SCA scan report.

The company I work for requires that all of our projects are scanned for security vulnerabilities before each deployment. So our continuous delivery pipelines fail even if one of our dependencies has a dependency that has a dependency with a known security vulnerability. And that's what happened here.

So my only option is to either report the vulnerability and try to get it patched, or remove the dependency. That's why I reported it.

I thought the maintainers would want to be aware that their library is showing in security scanners as having a vulnerability. 🤷

@SvenRtbg
Copy link
Contributor

@paulshryock Thanks for your response.

I am investigating this issue, and my current conclusion is that the other project mentioned in this report in fact has a serious problem when encountering JWT that have been tampered with. That may even shut the server down. To mitigate their anticipated shortcomings of this JsonMapper library, they forked and patched their version. And now for unknown reasons these security scanner results pop up claiming that THIS library here has a DoS vulnerability of high severity.

I am looking at the test suite, and I have to admit there is a bit of ambiguity present in the array test case (and to @cweiske: I'll consolidate and discuss my findings/proposed code changes, but that will need some more time), so it definitely allows to have null values in an array that it's PHPDoc claims to be of a defined other type only. But by itself, this isn't any severe vulnerability inside this library, it is much more likely a vulnerability outside due to not properly validating a JWT before using it.

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

No branches or pull requests

3 participants