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

Stack overflow error caused by serialization of List with cyclic dependency -- NOT CVE #3973

Closed
PoppingSnack opened this issue Jun 9, 2023 · 6 comments
Labels
need-test-case To work on issue, a reproduction (ideally unit test) needed

Comments

@PoppingSnack
Copy link

Stack overflow error caused by jackson serialization List

Description

jackson before v2.15.2 was discovered to contain a stack overflow via the List parameter. This vulnerability allows attackers to cause a Denial of Service (DoS) via a crafted string.

Error Log

Exception in thread "main" java.lang.StackOverflowError
	at com.fasterxml.jackson.core.json.JsonWriteContext.writeValue(JsonWriteContext.java:267)
	at com.fasterxml.jackson.core.json.WriterBasedJsonGenerator._verifyValueWrite(WriterBasedJsonGenerator.java:939)
	at com.fasterxml.jackson.core.json.WriterBasedJsonGenerator.writeStartArray(WriterBasedJsonGenerator.java:282)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:78)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:18)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serializeContents(IndexedListSerializer.java:119)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:79)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:18)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serializeContents(IndexedListSerializer.java:119)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:79)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:18)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serializeContents(IndexedListSerializer.java:119)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:79)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:18)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serializeContents(IndexedListSerializer.java:119)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:79)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:18)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serializeContents(IndexedListSerializer.java:119)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:79)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:18)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serializeContents(IndexedListSerializer.java:119)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:79)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:18)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serializeContents(IndexedListSerializer.java:119)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:79)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:18)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serializeContents(IndexedListSerializer.java:119)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:79)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:18)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serializeContents(IndexedListSerializer.java:119)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:79)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:18)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serializeContents(IndexedListSerializer.java:119)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:79)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:18)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serializeContents(IndexedListSerializer.java:119)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:79)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:18)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serializeContents(IndexedListSerializer.java:119)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:79)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:18)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serializeContents(IndexedListSerializer.java:119)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:79)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:18)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serializeContents(IndexedListSerializer.java:119)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:79)
	at com.fasterxml.jackson.databind.ser.impl.IndexedListSerializer.serialize(IndexedListSerializer.java:18)

PoC

        <dependency>
            <groupId>com.fasterxml.jackson.core</groupId>
            <artifactId>jackson-databind</artifactId>
            <version>2.15.2</version>
        </dependency>
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;

import java.util.ArrayList;

public class PoC3 {

    public static void main(String[] args) {
        ArrayList<Object> list = new ArrayList<>();
        list.add(list);

        ObjectMapper objectMapper = new ObjectMapper();
        try {
            String jsonString = objectMapper.writeValueAsString(list);
            System.out.println(jsonString);
        } catch (JsonProcessingException e) {
            e.printStackTrace();
        }
    }
}

Rectification Solution

  1. Refer to the solution of jackson-databind: Add the depth variable to record the current parsing depth. If the parsing depth exceeds a certain threshold, an exception is thrown. (fcfc499)

  2. Refer to the GSON solution: Change the recursive processing on deeply nested arrays or JSON objects to stack+iteration processing.((google/gson@2d01d6a20f39881c692977564c1ea591d9f39027))

References

  1. If the value in map is the map's self, the new new JSONObject(map) cause StackOverflowError which may lead to dos jettison-json/jettison#52
  2. https://github.com/jettison-json/jettison/pull/53/files
@PoppingSnack PoppingSnack added the to-evaluate Issue that has been received but not yet evaluated label Jun 9, 2023
@yawkat
Copy link
Member

yawkat commented Jun 9, 2023

#3972

@pjfanning
Copy link
Member

pjfanning commented Jun 9, 2023

Your POC does not have a crafted String. It has this input:

        ArrayList<Object> list = new ArrayList<>();
        list.add(list);

How can an attacker provide this input?

I'm not arguing that it would be nice for Jackson to police against this serialization issue but to go around trying to claim that this is a DoS vector is a bit much.

@cowtowncoder
Copy link
Member

Agreed with @pjfanning . Missing limit on serialization side is a known gap and we hope to address this (similar to reader-side max-nesting-depth). But there is no automated way for attacker to force generation of "too deep" nesting with 2.15 and later (due to added reader-side limit); nor can they force serialization.

@cowtowncoder cowtowncoder added need-test-case To work on issue, a reproduction (ideally unit test) needed and removed to-evaluate Issue that has been received but not yet evaluated labels Jun 12, 2023
@pjfanning
Copy link
Member

could be fixed by FasterXML/jackson-core#1055

@cowtowncoder cowtowncoder changed the title Stack overflow error caused by jackson serialization List Stack overflow error caused by serialization of List with cyclic dependency -- NOT CVE Jun 18, 2023
@pjfanning
Copy link
Member

pjfanning commented Jul 25, 2023

same issue as #3972

we do not accept that this is a security vulnerability

this 2.16 change will improve the handling of cyclic data - FasterXML/jackson-core#1055

@cowtowncoder
Copy link
Member

@pjfanning while that fix would alleviate any issue with cyclic data structures, you might want to rephrase that comment in this context so as not to suggest that the reported issue was valid. Issue is not valid given that the reproduction is invalid; no external attack is possible.

My concern is that folks evaluating vulnerabilities would read the comment and assume there was a legit vulnerability that is now pending for a fix to be released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need-test-case To work on issue, a reproduction (ideally unit test) needed
Projects
None yet
Development

No branches or pull requests

4 participants