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.

Sign up for GitHub

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

Jump to bottom

Stack Overflow (Criteria.parse) #973

Closed
PoppingSnack opened this issue Dec 11, 2023 · 19 comments · Fixed by #985
Closed

Stack Overflow (Criteria.parse) #973

PoppingSnack opened this issue Dec 11, 2023 · 19 comments · Fixed by #985
Labels
BUG

Comments

@PoppingSnack
Copy link

Stack Overflow (Criteria.parse)

Description

A stack overflow vulnerability exists in the Criteria.parse method in json-path 2.8.0. Specially crafted input can cause uncontrolled recursion, resulting in stack overflow.

Error Log

java.lang.StackOverflowError
	at java.base/java.util.Collections$SingletonList.<init>(Collections.java:4837)
	at java.base/java.util.Collections.singletonList(Collections.java:4823)
	at com.jayway.jsonpath.internal.path.PathTokenFactory.createSinglePropertyPathToken(PathTokenFactory.java:18)
	at com.jayway.jsonpath.internal.path.PathCompiler.readPropertyOrFunctionToken(PathCompiler.java:253)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:153)
	at com.jayway.jsonpath.internal.path.PathCompiler.readBracketPropertyToken(PathCompiler.java:634)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:137)
	at com.jayway.jsonpath.internal.path.PathCompiler.readPropertyOrFunctionToken(PathCompiler.java:256)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:153)
	at com.jayway.jsonpath.internal.path.PathCompiler.readBracketPropertyToken(PathCompiler.java:634)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:137)
	at com.jayway.jsonpath.internal.path.PathCompiler.readPropertyOrFunctionToken(PathCompiler.java:256)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:153)
	at com.jayway.jsonpath.internal.path.PathCompiler.readBracketPropertyToken(PathCompiler.java:634)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:137)
	at com.jayway.jsonpath.internal.path.PathCompiler.readPropertyOrFunctionToken(PathCompiler.java:256)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:153)
	at com.jayway.jsonpath.internal.path.PathCompiler.readBracketPropertyToken(PathCompiler.java:634)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:137)
	at com.jayway.jsonpath.internal.path.PathCompiler.readPropertyOrFunctionToken(PathCompiler.java:256)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:153)
	at com.jayway.jsonpath.internal.path.PathCompiler.readBracketPropertyToken(PathCompiler.java:634)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:137)
	at com.jayway.jsonpath.internal.path.PathCompiler.readPropertyOrFunctionToken(PathCompiler.java:256)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:153)
	at com.jayway.jsonpath.internal.path.PathCompiler.readBracketPropertyToken(PathCompiler.java:634)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:137)
	at com.jayway.jsonpath.internal.path.PathCompiler.readPropertyOrFunctionToken(PathCompiler.java:256)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:153)
	at com.jayway.jsonpath.internal.path.PathCompiler.readBracketPropertyToken(PathCompiler.java:634)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:137)
	at com.jayway.jsonpath.internal.path.PathCompiler.readPropertyOrFunctionToken(PathCompiler.java:256)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:153)
	at com.jayway.jsonpath.internal.path.PathCompiler.readBracketPropertyToken(PathCompiler.java:634)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:137)
	at com.jayway.jsonpath.internal.path.PathCompiler.readPropertyOrFunctionToken(PathCompiler.java:256)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:153)
	at com.jayway.jsonpath.internal.path.PathCompiler.readBracketPropertyToken(PathCompiler.java:634)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:137)
	at com.jayway.jsonpath.internal.path.PathCompiler.readPropertyOrFunctionToken(PathCompiler.java:256)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:153)
	at com.jayway.jsonpath.internal.path.PathCompiler.readBracketPropertyToken(PathCompiler.java:634)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:137)
	at com.jayway.jsonpath.internal.path.PathCompiler.readPropertyOrFunctionToken(PathCompiler.java:256)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:153)
	at com.jayway.jsonpath.internal.path.PathCompiler.readBracketPropertyToken(PathCompiler.java:634)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:137)
	at com.jayway.jsonpath.internal.path.PathCompiler.readPropertyOrFunctionToken(PathCompiler.java:256)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:153)
	at com.jayway.jsonpath.internal.path.PathCompiler.readBracketPropertyToken(PathCompiler.java:634)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:137)
	at com.jayway.jsonpath.internal.path.PathCompiler.readPropertyOrFunctionToken(PathCompiler.java:256)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:153)
	at com.jayway.jsonpath.internal.path.PathCompiler.readBracketPropertyToken(PathCompiler.java:634)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:137)
	at com.jayway.jsonpath.internal.path.PathCompiler.readPropertyOrFunctionToken(PathCompiler.java:256)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:153)
	at com.jayway.jsonpath.internal.path.PathCompiler.readBracketPropertyToken(PathCompiler.java:634)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:137)
	at com.jayway.jsonpath.internal.path.PathCompiler.readPropertyOrFunctionToken(PathCompiler.java:256)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:153)
	at com.jayway.jsonpath.internal.path.PathCompiler.readBracketPropertyToken(PathCompiler.java:634)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:137)
	at com.jayway.jsonpath.internal.path.PathCompiler.readPropertyOrFunctionToken(PathCompiler.java:256)
	at com.jayway.jsonpath.internal.path.PathCompiler.readNextToken(PathCompiler.java:153)

PoC

<dependency>
<groupId>com.jayway.jsonpath</groupId>
<artifactId>json-path</artifactId>
<version>2.8.0</version>
</dependency>
import com.jayway.jsonpath.Criteria;
import org.junit.Test;

public class CriteriaFuzzerParse {
    @Test
    public void parseFuzzerTest() {
        try {
            Criteria result = Criteria.parse("@[\"\",/\\");
        } catch (Exception e) {
        }
    }
}
@carnil
Copy link

carnil commented Dec 28, 2023

This seems to have recieved a CVE assigned: CVE-2023-51074

@skjolber
Copy link

skjolber commented Jan 2, 2024

@kallestenflo owasp dependency check flags this as a HIGH severity, any chance for a fix?

@blutorange
Copy link

blutorange commented Jan 3, 2024 •
edited
Loading

While I'm personally not certain whether a StackOverflowError really presents a HIGH severity, the stack overflow error also occurs even if you do not use the optional criteria filter feature, e.g.

JsonPath.read("[]","@[\"\",/\\") // no optional third argument with a filter predicate

JsonPath.compile("@[\"\",/\\")

So you would be affected by this CVE as long as you parse JSON paths from user inputs in any way.

@martinvisser martinvisser mentioned this issue Jan 9, 2024
Closed
@nunocenteio
Copy link

Hi. Our Analysis tools started rejecting our releases due to this vulnerability. Will it be fixed?

@mareknovotny
Copy link

mareknovotny commented Jan 10, 2024 •
edited
Loading

is this really CVE? @kallestenflo you can dispute it on CVE DB

just to be aware that @PoppingSnack reports very disputable CVEs see his activity https://github.com/PoppingSnack?tab=overview&from=2023-11-01&to=2023-11-30
i was popped by his report on mvel library where the problem was also not assessed as CVE at the end.

@sithmein
Copy link

I believe the CVE is in principle valid because iff you parsed a user-supplied JSON paths you will run into the issue. I seriously challenge the CVSS score, though. Any sane request handling framework will handle every request in a dedicated thread. Therefore the bug will only terminate this thread and no other requests will be impacted. Therefore the Availability Impact is at most "Low" leading to a CVSS of 5.3. I'm even arguing that it's "None" leading to a CVSS of 0.
In other programming languages a memory error will often terminate the complete process and then it's a "High" availability impact but not in Java.

@pollyshaw
Copy link

I am considering disputing it. It is a high vulnerability because of its high availability impact, which is defined as

There is total loss of availability, resulting in the attacker being able to fully deny access to resources in the impacted component; this loss is either sustained (while the attacker continues to deliver the attack) or persistent (the condition persists even after the attack has completed). Alternatively, the attacker has the ability to deny some availability, but the loss of availability presents a direct, serious consequence to the impacted component (e.g., the attacker cannot disrupt existing connections, but can prevent new connections; the attacker can repeatedly exploit a vulnerability that, in each instance of a successful attack, leaks a only small amount of memory, but after repeated exploitation causes a service to become completely unavailable).

I guess it would come down to that 'Alternatively...'. I guess if this function were exposed to the internet it could conceivably introduce a lot of long-running requests which hog memory. We might need evidence to show that this could not happen.

@sithmein
Copy link

Java has a default thread stack size of 2MB. The stack overflow happens within less than 10ms. Therefore I highly doubt that you can do anything harmful with it.

@nstrong-scw
Copy link

I agree with @sithmein here.

You only get to a HIGH impact if the web server handling the request is using some toy/bespoke HTTP server implementation that runs everything in a single thread. And maybe not even then.

Java's threading model uses a static amount of stack memory per thread, which means the memory consumption of N requests is the same regardless of whether the request is legitimate or not. Therefore, any memory exhaustion issues would be triggered by any spike in traffic and the bug has no impact on this.

Thread exhaustion? Unlike a regex attack (where a user-specified pattern might generate effectively-infinite matches and spike CPU usage), this is just a recursion bug and @sithmein reports it takes a tiny amount of time to trigger--probably faster than an actual legitimate request would take.

So, in truth, this is just a bug and should not be a CVE.

@karthickm512
Copy link

Does this CVE/bug also affects json-path version older than 2.8.0, say 2.7.0?

@raboof
Copy link

raboof commented Jan 11, 2024

Does this CVE/bug also affects json-path version older than 2.8.0, say 2.7.0?

Yes, 2.7.0 also produces a stack overflow for JsonPath.compile("@[\"\",/\\").

@ledex
Copy link

ledex commented Jan 12, 2024

The problem seems to be that indexOfNextSignificantChar returns -1 when the given character does not exist right of the given startPosition. This will become a problem since in the string provided in the example above we will search for a ] which does not exist. Therfore indexOfNextSignificantChar returns -1 and we will use this to set the new positon of the path. This can be seen here.

I think this might be the rootcause but I'll investigate further.

@twobiers twobiers mentioned this issue Jan 18, 2024
Merged
@twobiers
Copy link
Contributor

FYI in #985 I'll show a simple fix for this.

@noren95
Copy link

noren95 commented Jan 18, 2024

Hi, can someone from the maintainers confirm this is a valid or invalid CVE?

@ledex
Copy link

ledex commented Jan 18, 2024 •
edited
Loading

Sorry, I have not had time to have a closer look, but @twobiers' PR seems to fix this.
@noren95 IMO this is a valid CVE but it only affects you if you are using the deprecated Criteria.parse or Criteria.where. If you are already using Filter.parse you should not be affected and can ignore this CVE as far as I can tell.

@yeikel yeikel mentioned this issue Jan 18, 2024
Closed
@yeikel
Copy link

yeikel commented Jan 18, 2024 •
edited
Loading

Perhaps this is a good opportunity to remove these deprecated methods? As far as I can tell, they were deprecated 9 years ago

@ashirvadgupta
Copy link

Can someone clarify if json-path 2.7.0 is also affected?

@blutorange
Copy link

@ashirvadgupta Yes, see #973 (comment). You can verify it yourself by just running the code in the linked comment.

@prabhu
Copy link

prabhu commented Jan 24, 2024

I fail to understand why there is even a CVE against this. People should develop a hobby or go for a walk instead of seeking CVEs every day and minute.

@rgdoliveira rgdoliveira mentioned this issue Jan 25, 2024
Merged
@mike-lloyd03 mike-lloyd03 mentioned this issue Jan 29, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
BUG
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Check for the existence of the next significant bracket twobiers/JsonPath