GitHub user ppkarwasz created a discussion: Log4j 2.x audit against Oracle's Secure Coding Guidelines for Java SE
# Log4j 2.x audit against Oracle's Secure Coding Guidelines for Java SE This is an AI-assisted audit of the Log4j 2.x codebase against Oracle's *Secure Coding Guidelines for Java SE* (https://www.oracle.com/java/technologies/javase/seccodeguide.html), current edition (79 guidelines across 10 sections). We can use it to fix small bugs and introduce additional hardenings. Each finding below records a place where Log4j deviates from a guideline, in whole or in part. Findings are numbered `LOG4J-<n>`. Many are deliberate, documented trade-offs (configuration is inside the trust boundary, garbage-free logging sacrifices defensive copies, SecurityManager rules are moot on modern JVMs); those are marked accordingly. The line numbers are pointers to verify, not gospel. ## Scope: these are hardening notes, not vulnerabilities **None of these findings is an in-scope vulnerability under the [Apache Logging Services threat model](https://logging.apache.org/security.html).** They are secure-coding-guideline observations, code quality, defense-in-depth, and hardening opportunities, not undisclosed security holes. Every finding falls into a category the threat model explicitly places out of scope: trusted operator-controlled configuration, trusted developer-controlled structural inputs (logger names, levels, markers, format strings), code running in the same process (plugins, custom appenders, subclasses), active sinks that re-interpret well-formed output, unstructured layouts that make no injection guarantee, information disclosure and log masking (deployer/developer responsibilities), and Java deserialization (not part of normal operation; per the threat model, "bypasses are treated as opportunities for further hardening, not as vulnerabilities"). The single category the frameworks *do* commit to defending, log injection that breaks the structure of a **structured** layout (XML, JSON, RFC 5424, HTML) at a passive sink, has no open finding here: every such check came back Respected. Several findings *sound* like classic vulnerabilities (deserialization, XXE, JNDI). Those carry an inline **Threat model** note pointing at the specific provision that places them out of scope, so they are not mistaken for undisclosed vulnerabilities. This document is therefore suitable for public discussion (e.g. the `dev@` list) rather than private security reporting. ## Verdict tally | Verdict | Count | |------------------|-------| | Respected | 13 | | Mostly respected | 29 | | Partial | 25 | | Violated | 2 | | Not applicable | 10 | Guidelines with no deviation (Respected / Not applicable) are not listed as findings. They are: 0-4, 0-7, 1-4, 3-2, 3-3, 3-4, 3-6, 3-7, 3-8, 3-9, 5-2, 5-3, 6-4, 6-5, 8-2, 8-4, 8-5, 9-1, 9-6, 9-12, 9-15, 9-16. ## Priority findings The findings most worth acting on, independent of the SecurityManager being disabled and of the config-is-trusted threat model: - **LOG4J-40** - `MarkerManager.Log4jMarker` has no `readResolve`/`readObject`; a deserialized marker bypasses interning and validation. - **LOG4J-41** - On Java 8, `LogEventProxy` deserializes a `MarshalledObject`'s inner bytes through an unfiltered stream. - **LOG4J-31** - The deserialization allowlist (`SerializationUtil.REQUIRED_JAVA_CLASSES` / `REQUIRED_JAVA_PACKAGES`) is a mutable `Arrays.asList` view. - **LOG4J-17** - The log4j-1.2 bridge XML parser sets no XXE hardening features. - **LOG4J-37** - `StrSubstitutor` constructors call overridable public setters. - **LOG4J-15** - CSV layouts do not neutralize spreadsheet formula injection. --- ## Section 0: Fundamentals ### LOG4J-1 - Guideline 0-0 (FUNDAMENTALS-0): Prefer obviously no flaws over no obvious flaws `JndiManager.lookup` validates the name by constructing a `java.net.URI`, then passes the *original, unparsed* string to `context.lookup(name)` (`log4j-core/.../core/net/JndiManager.java:249-251`). This is the parser-discrepancy shape behind the CVE-2021-45046 bypass class. Also, TLS hostname verification defaults to `false` (`log4j-core/.../core/net/ssl/SslConfigurationFactory.java:89`). *Mitigated:* JNDI is disabled by default and restricted to the `java:` scheme. *Threat model:* out of scope. The JNDI name and the TLS settings both come from operator-controlled configuration, which is trusted; an adversary who can influence them is the out-of-scope "adversary able to modify configuration". Not an undisclosed vulnerability. ### LOG4J-2 - Guideline 0-1 (FUNDAMENTALS-1): Design APIs to avoid security concerns Log4j is a canonical example of retrofitted security: JNDI lookups, script support, and message-lookup expansion were designed into the API and later had to be gated behind system properties (`JndiManager.java:40-57`, `ScriptManager.java:73-96`). Extensibility also requires many non-final public classes in sensitive positions (`JndiManager`, `JndiLookup`). ### LOG4J-3 - Guideline 0-2 (FUNDAMENTALS-2): Avoid duplication log4j-core vendors copied third-party code that does not receive upstream fixes automatically: a full copy of picocli (`.../core/tools/picocli/CommandLine.java`), Commons-Lang-derived date formatting (`.../core/util/datetime/FastDateFormat.java:68`), and a `StrSubstitutor` forked from commons-text. log4j-1.2-api re-implements dozens of same-named classes as a compatibility bridge (intentional, but duplicated parsing logic). ### LOG4J-4 - Guideline 0-3 (FUNDAMENTALS-3): Restrict privileges *Mostly respected.* As a library Log4j cannot drop OS privileges, but dangerous features are off by default and separately toggled (`log4j2.enableJndi*`, `log4j2.Script.enableLanguages`, `log4j2.Configuration.allowedProtocols`). No action required; recorded for completeness. ### LOG4J-5 - Guideline 0-5 (FUNDAMENTALS-5): Minimise the number of security checks *Mostly respected, deliberate deviation.* `isJndiEnabled` re-reads properties on every call rather than caching ("to allow complex stacks to effect this setting", `JndiManager.java:55`), trading the minimize-checks rule for fail-safe re-validation, which errs in the safe direction. ### LOG4J-6 - Guideline 0-6 (FUNDAMENTALS-6): Encapsulate Public mutable fields exist in several inner helper types: `LoggerConfig.LevelAndRefs` (`.../core/config/LoggerConfig.java:1218-1219`), `MutableThreadContextMapFilter` (`.../core/filter/MutableThreadContextMapFilter.java:508-509`), `CronExpression.ValueSet` (`.../core/util/CronExpression.java:1688-1690`), and many in vendored picocli. Most are helper types rather than security-bearing state. ### LOG4J-7 - Guideline 0-8 (FUNDAMENTALS-8): Secure third-party code The vendored copies from LOG4J-3 (picocli, FastDateFormat, StrSubstitutor) are outside Dependabot's reach and must be patched by hand, which is exactly the update-lag risk this guideline warns about. Dependency hygiene otherwise strong (Dependabot across branches, OSS-Fuzz, 16 of 19 core dependencies optional). --- ## Section 1: Denial of Service ### LOG4J-8 - Guideline 1-1 (DOS-1): Beware of disproportionate resource use PatternLayout output is uncapped by default (`%maxLen` / `RegexReplacement` are opt-in), and config-supplied regexes (`%replace`, `RegexFilter`) run against attacker-controlled message text, so catastrophic backtracking is possible if the deployer writes a vulnerable pattern. Log message size itself is unbounded by design. *Mitigated:* cyclic-interpolation detection, reusable-buffer trimming, layout builder caps, JsonTemplateLayout truncation, and `ThrowableProxy` cycle guards bound most other costs. ### LOG4J-9 - Guideline 1-2 (DOS-2): Release resources in all cases try-with-resources is the dominant idiom, but resource leaks did slip through: commit `f10c986684` had to add `HttpURLConnection.disconnect()` on failure paths in `ConfigurationSource.getConfigurationSource(URL)`. `LoggerContextAdmin.java:138` also hands an open `configURL.openStream()` to a `ConfigurationSource` whose closing depends on the consumer. ### LOG4J-10 - Guideline 1-3 (DOS-3): Resource limit checks free of integer overflow `Integers.ceilingNextPowerOfTwo` (`.../core/util/Integers.java:64`, `1 << (32 - nlz(x-1))`) overflows to a negative value for inputs above 2^30, and `DisruptorUtil.calculateRingBufferSize` (`.../core/async/DisruptorUtil.java:88-102`) enforces only a minimum, not a maximum, before calling it. *Mitigated:* the bad value is caught later by the Disruptor's own validation, and the input is trusted configuration. ### LOG4J-11 - Guideline 1-5 (DOS-5): Avoid using user input as hash keys `DefaultThreadContextMap.toMap()` copies context into a `HashMap` (`.../spi/DefaultThreadContextMap.java:157-172`), and logger/manager registries are HashMap/ConcurrentHashMap keyed by names. *Mitigated:* keys are developer/config-controlled Strings, and JDK 8+ HashMap tree bins bound collision attacks at O(log n). Per-event user data uses array-backed maps (`SortedArrayStringMap`, `UnmodifiableArrayBackedMap`) that avoid hashing. --- ## Section 2: Confidential Information ### LOG4J-12 - Guideline 2-1 (CONFIDENTIAL-1): Purge sensitive information from exceptions Log4j embeds file-system paths and config URLs in exceptions and status messages: `FileUtils.mkdir` throws with `dir.getAbsolutePath()` (`.../core/util/FileUtils.java:119,129`), `FilePasswordProvider` names the password file (`.../core/net/ssl/FilePasswordProvider.java:61,78`), `ConfigurationSource` logs full URLs on failure. *Mitigated:* these describe deployer-supplied paths and go to the StatusLogger, not end users; but the library does not sanitize before propagating, so a caller embedding these in a response would leak layout information. ### LOG4J-13 - Guideline 2-2 (CONFIDENTIAL-2): Do not log highly sensitive information `AbstractDriverManagerConnectionSource.Builder.password` (`.../core/appender/db/jdbc/AbstractDriverManagerConnectionSource.java:56`) is not marked `@PluginBuilderAttribute(sensitive = true)`, and its `toString()` returns the raw `connectionString` which may embed credentials in a JDBC URL (lines 229-231). *Mitigated:* the password is `char[]`, so it prints as an object hash in debug output. The `sensitive = true` mechanism is correctly applied to SMTP, JMS, keystore, and Cassandra passwords. ### LOG4J-14 - Guideline 2-3 (CONFIDENTIAL-3): Purge sensitive information from memory `SmtpManager` keeps the SMTP password as an immutable `String` inside a JavaMail `PasswordAuthentication` for the manager's lifetime (`.../core/appender/mom/SmtpManager.java:103,328-336`); `StoreConfiguration.equals`/`hashCode` create password clones that are never zeroed and a deprecated `String getPassword()` remains (`.../core/net/ssl/StoreConfiguration.java:77,106-129`); the `char[]` clone in `SslConfiguration.loadKeyManagers` is not wiped. *Mitigated:* the SSL keystore path itself zeroes passwords in `finally`. --- ## Section 3: Injection and Inclusion ### LOG4J-15 - Guideline 3-1 (INJECT-1): Generate valid formatting CSV layouts (`.../core/layout/AbstractCsvLayout.java`) apply only CSV quoting, with no neutralization of spreadsheet formula injection (leading `=`, `+`, `-`, `@`), and the manual carries no warning about it. PatternLayout emits raw, unescaped message text by default; CRLF/control-character injection into text logs is prevented only if the user opts into `%enc`/`%replace`. See also LOG4J-16. *Threat model:* out of scope. Spreadsheet formula injection targets an **active sink** (a spreadsheet that evaluates formula syntax), named explicitly as out of scope; and Pattern Layout is an **unstructured layout**, which makes no injection guarantee. A documentation note about the CSV case would still be a courtesy improvement, but it is not a vulnerability. ### LOG4J-16 - Guideline 3-1 (INJECT-1): CSV formula injection (cross-reference) Spreadsheet formula injection in CSV layouts, tracked under LOG4J-15. Listed separately because it is the most concrete, fixable item under this guideline: neutralize leading `= + - @` or document the risk in `layouts.adoc`. *Threat model:* out of scope, as active-sink re-interpretation; see LOG4J-15. A hardening or documentation courtesy, not a vulnerability. ### LOG4J-17 - Guideline 3-5 (INJECT-5): Restrict XML inclusion Two items. (a) XInclude is enabled by default for the core config document (`.../core/config/xml/XmlConfiguration.java:185-235`), so a config file can pull in arbitrary local files (acceptable only because config is trusted). (b) The log4j-1.2 bridge parser (`log4j-1.2-api/.../org/apache/log4j/xml/XmlConfiguration.java:151-172`) uses a validating builder whose `Log4jEntityResolver` substitutes the bundled `log4j.dtd` but disables no external-entity features, so a hostile v1 config file could trigger external entity resolution. The core parser, by contrast, disables DTDs and all external-entity features. *Threat model:* out of scope. Both the XInclude and the bridge-XXE items require the adversary to control a **configuration file**, which is a trusted operator-controlled source; "an adversary able to modify ... configuration files" is explicitly out of scope. Despite reading as "XXE", this is not an undisclosed vulnerability. Hardening the bridge parser to match the core parser is still worthwhile defense-in-depth. --- ## Section 4: Accessibility and Extensibility ### LOG4J-18 - Guideline 4-1 (EXTEND-1): Limit accessibility of classes, interfaces, methods, fields `org.apache.logging.log4j.util` is a public exported package whose own `package-info.java` says use "is not supported", enforced only by `@InternalApi` javadoc. Many implementation classes in exported packages are public non-final (`Interpolator`, `ConfigurationSource`, `SslConfiguration`). JAR packages are not sealed (no `Sealed:` manifest entries). ### LOG4J-19 - Guideline 4-2 (EXTEND-2): Use modules to hide internal packages The OSGi `Export-Package` unconditionally exports `org.apache.logging.log4j.util.internal` (api) and `core.util.internal.instant` (core), more permissive than the BND-generated JPMS descriptors, which export `util.internal` only on a qualified basis. Core's JPMS descriptor also exports ~50 packages (essentially the whole implementation), so broad-implementation encapsulation is weak by design (plugin architecture). ### LOG4J-20 - Guideline 4-4 (EXTEND-4): Limit exposure of ClassLoader instances `LoaderUtil` publicly exposes `getClassLoader()` and `getThreadContextClassLoader()` (`.../util/LoaderUtil.java:92,146`) plus by-name instantiation via TCCL, and `Loader` (`.../core/util/Loader.java`) re-exposes the same, both in exported packages. Nothing prevents third-party code from using these to reach loaders it should not have. *Mitigated:* the package is documented internal, an `log4j.ignoreTCL` opt-out exists, and TCCL use is inherent to plugin discovery. ### LOG4J-21 - Guideline 4-5 (EXTEND-5): Limit the extensibility of classes and methods 365 files declare non-final public classes, including security-sensitive ones: `JndiManager`, `JndiLookup`, `Interpolator`, `StrSubstitutor`, `ConfigurationSource`, `SslConfiguration`, `Log4jLogEvent`. No `sealed` types (Java 8 source target). ~210 files do correctly use `public final class`. ### LOG4J-22 - Guideline 4-6 (EXTEND-6): Understand how a superclass can affect subclass behavior Deep inheritance chains such as `JndiManager extends AbstractManager` and `StatusLogger extends AbstractLogger` mean superclass evolution (new methods on the public, evolving `AbstractManager`/`AbstractLogger`) can silently change subclass behavior. Accepted as API design rather than a defended boundary. --- ## Section 5: Input Validation ### LOG4J-23 - Guideline 5-1 (INPUT-1): Validate inputs Pattern layouts do not escape CRLF or other control characters in untrusted messages by default (log injection is left to the user via `%enc`, a documented threat-model decision; see LOG4J-15). `FileUtils.fileFromUri` (`.../core/util/FileUtils.java:60`) accepts arbitrary paths, relying on config being trusted. Configuration input is otherwise validated via a constraint framework (`@Required`, `@NotBlank`, `@ValidHost`, `@ValidPort`). ### LOG4J-24 - Guideline 5-4 (INPUT-4): Verify API behavior related to input validation `JndiManager.lookup` validates the name by constructing a `java.net.URI` but performs `context.lookup(name)` on the original unparsed string (`.../core/net/JndiManager.java:249-251`), relying on `java.net.URI` and the JNDI name parser agreeing (the exact anti-pattern this guideline describes; same root cause as LOG4J-1). SSL callers must also know that `verifyHostName` / `log4j2.sslVerifyHostName` default to `false` rather than assuming the SSL API verifies hostnames. *Mitigated:* the JNDI path is disabled by default and restricted to null/`java:` schemes. *Threat model:* out of scope, same basis as LOG4J-1: the JNDI name and the SSL settings are operator-controlled configuration (trusted). Not an undisclosed vulnerability. --- ## Section 6: Mutability ### LOG4J-25 - Guideline 6-2 (MUTABLE-2): Create copies of mutable output values `ParameterizedMessage.getParameters()` (`.../message/ParameterizedMessage.java:218-220`), `ObjectArrayMessage`/`StringFormattedMessage.getParameters()`, and `ThrowableProxy.getExtendedStackTrace()`/`getSuppressedProxies()` (`.../core/impl/ThrowableProxy.java:337-339,448-450`) return internal arrays uncopied. `Log4jLogEvent.getInstant()` returns its internal `MutableInstant`, castable back to the mutable type. *Deliberate:* allocation avoidance on the hot logging path. ### LOG4J-26 - Guideline 6-3 (MUTABLE-3): Create safe copies of mutable and subclassable input values `ParameterizedMessage` stores the caller's `Object[] args` without copying (`.../message/ParameterizedMessage.java:139-144`), so caller mutation after the log call is observable (a TOCTOU window, especially with async logging); `ObjectMessage`/ `ObjectArrayMessage` store raw references. *Deliberate:* the documented pass-through design of the Message API (LOG4J2-1688), mitigated by prompt formatting and async mementos. ### LOG4J-27 - Guideline 6-6 (MUTABLE-6): Treat passing input to untrusted object as output Synchronous appenders and layouts receive the live `LogEvent`, including the internal `StringMap` context data and, in garbage-free mode, the shared `MutableLogEvent`. Async paths snapshot first via `Log4jLogEvent.createMemento` (`.../core/appender/AsyncAppender.java:165-167`). *Deliberate:* the threat model treats plugins as trusted code; a performance trade-off. ### LOG4J-28 - Guideline 6-7 (MUTABLE-7): Treat output from untrusted object as input Arrays returned by a user-implemented `Message.getParameters()` are consumed without copying, consistent with the pass-through Message design (LOG4J-26). Throwable data is otherwise eagerly copied into owned snapshots, and deserialized input is treated as hostile. ### LOG4J-29 - Guideline 6-8 (MUTABLE-8): Define wrapper methods around modifiable internal state Bare public mutable fields without wrapper or validation: `ThreadContextDataInjector.contextDataProviders` (`.../core/impl/ThreadContextDataInjector.java:67`), `DefaultLoggerContextAccessor.INSTANCE` (`.../core/DefaultLoggerContextAccessor.java:27`), and a raw protected static map `ClassLoaderContextSelector.CONTEXT_MAP` (`.../core/selector/ClassLoaderContextSelector.java:54`) exposed to subclasses. ### LOG4J-30 - Guideline 6-9 (MUTABLE-9): Make public static fields final Three non-final public static fields: `DefaultLoggerContextAccessor.INSTANCE` (freely reassignable by any caller), `ThreadContextDataInjector.contextDataProviders` (intentional OSGi extension point), and `StaticLoggerBinder.REQUESTED_API_VERSION` (`log4j-slf4j-impl/.../org/slf4j/impl/StaticLoggerBinder.java:35`, non-final by SLF4J binding contract). The first two overlap LOG4J-29. ### LOG4J-31 - Guideline 6-10 (MUTABLE-10): Ensure public static final field values are constants `XmlConfigurationFactory.SUFFIXES` is a public static final mutable `String[]` (`.../core/config/xml/XmlConfigurationFactory.java:36`; the JSON and YAML equivalents are correctly private). `SerializationUtil.REQUIRED_JAVA_CLASSES`/`REQUIRED_JAVA_PACKAGES` are public static final `Arrays.asList` views whose elements can be overwritten via `set()`, and they are security-relevant (the deserialization allowlist), though in an `internal` package (`.../util/internal/SerializationUtil.java:77,92`). `CronExpression.monthMap`/`dayMap` are protected static final mutable `HashMap`s. **The mutable allowlist is the priority item for this guideline (listed under Priority findings).** ### LOG4J-32 - Guideline 6-11 (MUTABLE-11): Do not expose mutable statics The ambient-logging architecture relies on mutable statics by design: `StatusLogger.InstanceHolder.INSTANCE` (replaceable via public `setLogger`), the entire `ThreadContext` static facade, and `Log4jLogEvent`'s static `CLOCK`/`nanoClock` with public `setNanoClock`. The guideline is knowingly traded away; the two bare public fields from LOG4J-29 additionally lack even singleton boilerplate. ### LOG4J-33 - Guideline 6-12 (MUTABLE-12): Do not expose modifiable collections `AbstractConfiguration.getProperties()` returns the live internal `propertyMap` `ConcurrentHashMap` that also backs the `Interpolator` used for property substitution (`.../core/config/AbstractConfiguration.java:137-138,177-179`), so callers can mutate lookup results; `getAppenders()` returns the internal `ConcurrentMap` field directly. Sibling getters (`getLoggers`, `getCustomLevels`) correctly return unmodifiable views. The properties map is arguably an intentional mutable API but is undocumented as such on `Configuration.getProperties()`. --- ## Section 7: Object Construction ### LOG4J-34 - Guideline 7-1 (OBJECT-1): Avoid exposing constructors of sensitive classes `StatusLogger` gained a public constructor in 2.23.0 (`.../status/StatusLogger.java:548`) plus a public static `setLogger` (line 576) that lets any code replace the process-wide singleton. `LoggerContext` has four public constructors; `StrSubstitutor` has ~10. The sensitive JNDI path is exemplary by contrast (private constructor, gated factories). ### LOG4J-35 - Guideline 7-2 (OBJECT-2): Prevent the unauthorized construction of sensitive classes `StatusLogger.setLogger` and its public constructor allow any code on the classpath to substitute the global status logger with no check (overlaps LOG4J-34). `LogManager` has only a `protected LogManager()` so it is subclassable by anyone (though it holds no per-instance state). *Note:* guideline moved to 9-18; SecurityManager-based, largely obsolete. `JndiManager` construction is genuinely gated by the `log4j2.enableJndi*` flags. ### LOG4J-36 - Guideline 7-3 (OBJECT-3): Defend against partially initialized instances of non-final classes No systematic defense (no initialized-flag idiom, no `this-escape` audit). Non-final sensitive classes with argument-validating constructors (`ConfigurationSource`, `SslConfiguration`) remain theoretically open to finalizer-attack-style partially-initialized instances. *Mitigated:* lifecycle state machines approximate the "check init state before use" advice, and sensitive behavior is gated at use time, not construction. ### LOG4J-37 - Guideline 7-4 (OBJECT-4): Prevent constructors from calling methods that can be overridden **VIOLATED.** `StrSubstitutor` (public, non-final, security-relevant post-CVE-2021-44228) has constructors that call its own public overridable setters (`.../core/lookup/StrSubstitutor.java:305-311`: `setVariableResolver`, `setVariablePrefix`, `setVariableSuffix`, `setEscapeChar`), letting a subclass observe `this` before initialization completes. `AbstractConfiguration`'s constructor calls `setState` (`.../core/config/AbstractConfiguration.java:163`), where `setState` is protected non-final in `AbstractLifeCycle`. `AbstractLogger` and builder-based classes avoid this. ### LOG4J-38 - Guideline 7-5 (OBJECT-5): Defend against cloning of non-final classes No sensitive class defends with a final `clone()` throwing `CloneNotSupportedException`. Because `JndiManager`, `ConfigurationSource`, `LogManager`, `StatusLogger` are non-final (LOG4J-21), a malicious subclass could add `Cloneable` and shallow-copy state. *Mitigated:* such clones only affect adversary-created instances; `Cloneable` is otherwise almost absent from production code. --- ## Section 8: Serialization and Deserialization > **Threat model (applies to every finding in this section).** Log4j does not > deserialize data as part of normal operation. The threat model states > plainly: "We provide **no guarantee** that deserializing a stream containing > classes from these projects is safe, regardless of the source," that > allowlist filtering "is **not** sufficient to make deserialization safe," and > that the shipped hardening utilities are "**partial** and **not exhaustive**; > bypasses are treated as opportunities for further hardening, **not** as > vulnerabilities in the project." The application performing deserialization > is responsible for ensuring the byte stream comes from a trusted source. > Everything below is therefore a *hardening opportunity for the optional > `FilteredObjectInputStream` helper*, not an undisclosed vulnerability, even > where the wording sounds like one (LOG4J-40, LOG4J-41). ### LOG4J-39 - Guideline 8-1 (SERIAL-1): Avoid serialization for security-sensitive classes `Message extends Serializable` (`.../message/Message.java:46`) and `Marker extends Serializable` (`.../Marker.java:28`) force the whole message/marker hierarchies to be serializable. *Mitigated:* the dangerous parts are gone: no network deserialization sink remains in 2.x, JMS appenders only produce, `SerializedLayout` is `@Deprecated` with a warning, and `core.Logger` serializes as a name-only proxy. ### LOG4J-40 - Guideline 8-3 (SERIAL-3): View deserialization the same as object construction Three gaps. (a) `MarkerManager.Log4jMarker` (`.../MarkerManager.java:112`) has no `readObject`/`readResolve`, so a deserialized marker bypasses `MarkerManager` interning and its name/parents arrive unvalidated (name can even be null). **Priority item.** (b) `LogEventProxy.readResolve` (`.../core/impl/Log4jLogEvent.java:1243-1265`) performs no validation or defensive copying, and its fields are interface-typed rather than final classes. (c) `StructuredDataId` validates name length and `@` in constructors but has no `readObject` duplicating those checks. Strong examples exist elsewhere (`Log4jLogEvent`/`MutableLogEvent`/ `RingBufferLogEvent` reject direct deserialization; `Level.readResolve` re-interns). *Threat model:* out of scope as a vulnerability (see the section banner). Reachable only by an application that chose to deserialize an untrusted stream, which the threat model assigns to the application; "priority item" here means priority *hardening*, not a security defect. ### LOG4J-41 - Guideline 8-6 (SERIAL-6): Filter untrusted serial data Three gaps. (a) The allowlist is coarse: whole package prefixes `java.lang.`, `java.time.`, `java.util.`, `org.apache.logging.log4j.` and `java.rmi.MarshalledObject` are allowed (`.../util/internal/SerializationUtil.java:81,92-93`). (b) **On Java 8, `LogEventProxy` stores the message as a `MarshalledObject` whose `get()` in `readResolve` deserializes attacker-supplied inner bytes with a plain unfiltered stream** (`Log4jLogEvent.java:1225-1231, 1267-1276`); JDK 9+ propagates the stream filter into `MarshalledObject`, JDK 8 does not. **Priority item.** (c) `assertFiltered` on JDK 9+ accepts any `ObjectInputStream` even when no filter is installed, leaving top-level stream filtering to the consuming application (only wrapped inner objects are force-filtered). *Threat model:* out of scope as a vulnerability (see the section banner). This is exactly the "bypass of the partial, non-exhaustive hardening utilities" the threat model declares to be a hardening opportunity, not a project vulnerability; the Java 8 `MarshalledObject` gap is worth closing but only strengthens a helper the application opts into. --- ## Section 9: Access Control Most of this section presumes a SecurityManager, disabled/deprecated on the JVMs Log4j 2.x targets, so verdicts are about the spirit of the rules. ### LOG4J-42 - Guideline 9-2 (ACCESS-2): Beware of callback methods `DefaultShutdownCallbackRegistry` (`.../core/util/DefaultShutdownCallbackRegistry.java:130,164`) accepts arbitrary caller-provided `Runnable`s and runs them on a JVM shutdown-hook thread without capturing the registrant's access control context; the 9-6 context-transfer pattern is not implemented anywhere. *Mitigated:* in typical deployments all registrants are application code, and the SM is disabled. ### LOG4J-43 - Guideline 9-3 (ACCESS-3): Safely invoke doPrivileged `ScriptManager` (`.../core/script/ScriptManager.java:203,228`) runs `AccessController.doPrivileged` around compilation and execution of script text taken from the configuration file, with caller-supplied `Bindings`, so configuration-controlled code executes with Log4j's full privileges in a large (non-minimal) privileged block. The tainted-input shape this guideline warns about. *Mitigated:* scripting is dead code unless the administrator opts in via `log4j2.Script.enableLanguages`, and config is trusted. *Threat model:* out of scope. The script text is part of the operator-controlled configuration (trusted), and an adversary who can supply it is the out-of-scope config-modifying adversary; script execution is also same-process code, which "shares the same trust level as the logging framework itself". Not an undisclosed vulnerability. ### LOG4J-44 - Guideline 9-4 (ACCESS-4): Know how to restrict privileges through doPrivileged **VIOLATED (in the letter).** No reduced-privilege contexts anywhere: no `AccessControlContext`, no `ProtectionDomain`, no limited `doPrivileged` overloads; `ScriptManager` (LOG4J-43) would be the natural place to restrict privileges when executing config-supplied scripts and instead grants Log4j's full privileges. *Mitigated:* low severity today (SM disabled, scripting off by default). ### LOG4J-45 - Guideline 9-5 (ACCESS-5): Be careful caching results of potentially privileged operations `LoaderUtil.GET_CLASS_LOADER_DISABLED` (`.../util/LoaderUtil.java:59-80`) is a globally cached `LazyBoolean` computed partly from the first caller's access control context, then reused for every later caller in every context, with no `checkPermission` guarding the cached path. ### LOG4J-46 - Guideline 9-7 (ACCESS-7): Understand how thread construction transfers context `Log4jThreadFactory` is SM-aware and runs only Log4j-internal Runnables, but `DefaultShutdownCallbackRegistry` (LOG4J-42) constructs its hook thread in whatever context performs Log4j initialization and later runs caller-registered Runnables on it, so third-party callbacks inherit that construction context. ### LOG4J-47 - Guideline 9-8 (ACCESS-8): APIs that bypass SecurityManager checks via the immediate caller's class loader Public `@InternalApi` utilities `LoaderUtil.getClassLoader()`/`getThreadContextClassLoader()` and `StackLocatorUtil.getCallerClass` (backed by caller-sensitive `sun.reflect.Reflection.getCallerClass`, `.../util/StackLocator.java:165-168`) return `ClassLoader` and `Class` objects to any caller, propagating results of caller-sensitive APIs outward. Log4j also loads config-named classes as a deputy. *Mitigated:* config is trusted; log-message content no longer drives class loading post-2.17. ### LOG4J-48 - Guideline 9-9 (ACCESS-9): APIs that perform tasks using the immediate caller's class loader One-argument `Class.forName` with configuration-supplied names in `AbstractDriverManagerConnectionSource.java:209` (JDBC driver from config) and `Loader.java:269` (fallback), plus caller-sensitive `DriverManager.getConnection` with config-supplied connection strings (`AbstractDriverManagerConnectionSource.java:158-160`). *Mitigated:* all inputs come from trusted configuration. ### LOG4J-49 - Guideline 9-10 (ACCESS-10): APIs that perform Java language access checks against the immediate caller `setAccessible` on members of configuration-selected plugin classes: `PluginBuilder.java:175,235` and `ReflectionUtil.java:58,74`, plus `UnsafeUtil.java:44` on a hardcoded cleaner method. Log4j exercises its own package-access capabilities on behalf of whatever wrote the configuration. *Mitigated:* members are self-obtained from config-named classes and never handed back out. ### LOG4J-50 - Guideline 9-11 (ACCESS-11): Method.invoke is ignored for checking the immediate caller Reflective invocation is core to the plugin system. Sharpest case: `FactoryMethodConnectionSource` (`.../core/appender/db/jdbc/FactoryMethodConnectionSource.java:92-103`) reflectively invokes an arbitrary static no-arg method whose class and name both come from configuration attributes, so with `Method.invoke` transparent to caller-sensitive checks, a malicious config could aim it at caller-sensitive JDK statics with Log4j as the effective caller. *Mitigated:* config is trusted. *Threat model:* out of scope. The class and method names are configuration attributes (trusted, operator-controlled); "a malicious config" is precisely the out-of-scope config-modifying adversary. Despite reading as arbitrary-method-invocation, not an undisclosed vulnerability. ### LOG4J-51 - Guideline 9-13 (ACCESS-13): Avoid returning the results of privileged operations `LoaderUtil.getThreadContextClassLoader()`/`getClassLoader()` (`.../util/LoaderUtil.java:92-113,146-152`) obtain `ClassLoader` references inside `AccessController.doPrivileged` and return them to any caller, so code lacking `RuntimePermission("getClassLoader")` can obtain loaders through Log4j. `StackLocatorUtil` similarly exposes caller `Class` objects. (Overlaps LOG4J-47.) No `Method`/`MethodHandle`/ `Lookup` objects are returned. ### LOG4J-52 - Guideline 9-14 (ACCESS-14): APIs that perform tasks using the immediate caller's module *Mostly respected.* `ServiceLoaderUtil` deliberately requires the caller to construct its own `ServiceLoader` so the load happens with the caller's module/loader; no `addExports`/`addOpens`/`privateLookupIn` in production code. Recorded for completeness. ### LOG4J-53 - Guideline 9-17 (ACCESS-17): Isolate unrelated code Per-classloader logger contexts provide good isolation, but global mutable state is written without isolation: `System.setProperty` calls in `ConfigurationFactory.java:488`, `osgi/Activator.java:69`, `util/UuidUtil.java:121`, and `PropertiesUtil.java:511`, plus process-wide singletons (`LogManager` factory, `StatusLogger`) shared across all contexts (overlaps LOG4J-32). ### LOG4J-54 - Guideline 9-18 (ACCESS-18): Prevent the unauthorized construction of sensitive classes No Log4j constructor or factory performs a SecurityManager check. Genuinely sensitive classes gate behavior by global opt-in configuration rather than construction checks (JNDI flags, script allowlist), so constructing the object grants nothing by itself. `FilteredObjectInputStream` is public and subclassable, relying on the JDK's inherited subclass-audit check rather than a Log4j check. ### LOG4J-55 - Guideline 9-19 (ACCESS-19): Defend against partially initialized instances of non-final classes No initialized-flag / `this(check())` / pimpl patterns; many public non-final classes can throw from constructors after partial initialization (overlaps LOG4J-36). *Mitigated:* the only `finalize()` override in production code is an intentionally empty 1.x compatibility stub (`log4j-1.2-api/.../org/apache/log4j/AppenderSkeleton.java:71`), and sensitive operations are gated at use time. ### LOG4J-56 - Guideline 9-20 (ACCESS-20): Security permissions of serialization and deserialization The message-class `readObject` methods (`ObjectMessage.java:132`, `ObjectArrayMessage.java:120-123`, `ParameterizedMessage.java:384`) call `in.readObject()` and rely on the caller supplying a filtered stream rather than enforcing it themselves (overlaps LOG4J-41c). The "deserialize with least privilege" doPrivileged aspect is moot without a SecurityManager. GitHub link: https://github.com/apache/logging-log4j2/discussions/4191 ---- This is an automatically sent email for [email protected]. To unsubscribe, please send an email to: [email protected]
