Limit the nesting depth of `JsonReader` and document it as internal

Abierto
  • ¿Algo no está bien?
#4,463 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 1 día

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
4/5
Tiempo estimado
3-5 días
Aptitud para principiantes
55/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
java
Área
backend

Línea de trabajo

Start with JsonReader in the 2.x and 3.x packages named in the issue, then inspect JsonTemplateLayoutCodecFuzzer and its existing tests. Add coverage for input one nesting level above the proposed limit and check the fuzzer’s input bound. Done means excessive nesting raises IllegalArgumentException, the fuzzer no longer expects stack overflows, and Javadocs describe the parser as internal and for trusted input; the package move is only a consideration.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

configuration enhancement layouts

Log4j contains a minimal JSON parser, JsonReader, used to parse Log4j's own configuration: JSON configuration files, the MutableThreadContextMapFilter configuration, property mappings, and JSON Template Layout templates and resolver configuration. In 2.x it lives in org.apache.logging.log4j.layout.template.json.util; in 3.x in org.apache.logging.log4j.kit.json, which log4j-kit exports (OSGi @Export, version 3.0.0) under the module description "Helpers to implement Log4j 2.x API".

The parser recurses through readToken(), readArray() and readObject() without a depth limit, so a deeply nested document (a few thousand levels with the default thread stack) ends in a StackOverflowError instead of the IllegalArgumentException thrown for other malformed input. JsonTemplateLayoutCodecFuzzer currently works around this by bounding the length of its input (see OSS-Fuzz issue 370272570). Moreover, nothing in the Javadoc says that the parser is meant only for Log4j's own, operator-supplied input, and the exported 3.x package can give the impression that it is a general-purpose JSON parser.

Proposal:

  1. Add a nesting-depth counter to JsonReader, incremented on entry to readArray() and readObject(), and throw an IllegalArgumentException once it exceeds a fixed limit (e.g. 128, far above the nesting of any real configuration file), in both 2.x and 3.x. Add a test parsing a document one level above the limit.
  2. Relax the input length bound of JsonTemplateLayoutCodecFuzzer and remove its comment about expected stack overflows.
  3. State in the Javadoc of JsonReader and of its package that it is an internal component intended only for trusted input. Before 3.0.0 GA, consider moving the 3.x class to a non-exported (internal) package.

This issue originates from a private security report classified as a hardening.

Lenguaje dominante
Java
Estrellas
3.6k
Forks
1.8k
Merge medio
5 d 6 h
PR fusionados (30 d)
24

Preparar el entorno

  • Sin Dockerfile ni archivo de Docker Compose
  • Tiene una plantilla de pull request
  • Sin guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de apache/logging-log4j2

Todos los issues de apache/logging-log4j2

Issues similares

Más issues de Java

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.