This is an automated email from the ASF dual-hosted git repository.
garydgregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-lang.git
The following commit(s) were added to refs/heads/master by this push:
new 2e16df626 Attacker-controlled exception message forges frames in
getRootCauseStackTrace output AND suppresses all real frames, beyond cosmetic
log spoofing (f025).
2e16df626 is described below
commit 2e16df6260fb207cf45ae456e25e391cb7ef9bf8
Author: Gary Gregory <[email protected]>
AuthorDate: Sat Sep 5 15:26:19 2026 -0400
Attacker-controlled exception message forges frames in
getRootCauseStackTrace output AND suppresses all real frames, beyond
cosmetic log spoofing (f025).
---
src/changes/changes.xml | 1 +
.../commons/lang3/exception/ExceptionUtils.java | 91 ++++++++++++++++++++--
.../lang3/exception/ExceptionUtilsTest.java | 91 ++++++++++++++++++++++
3 files changed, 178 insertions(+), 5 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 228772b45..5f075851e 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -274,6 +274,7 @@ java.lang.NullPointerException: Cannot invoke
<action type="fix" dev="ggregory" due-to="Gary
Gregory">StringEscapeUtils.escapeHtml4/3 apostrophe gap: single-quoted and
unquoted HTML attribute contexts are trivially breakable, and the javadoc
discloses the gap only as an HTML4 entity-legality footnote (f022).</action>
<action type="fix" dev="ggregory" due-to="Gary
Gregory">ArrayUtils.addAll/insert length arithmetic overflows (undeclared
NegativeArraySizeException) while sibling concat is overflow-checked
(f023).</action>
<action type="fix" dev="ggregory" due-to="Gary
Gregory">FormattableUtils.append pads right-justified output with insert(0) per
char; O(width^2); '%500000s' costs ~1.25e11 char moves (f024).</action>
+ <action type="fix" dev="ggregory" due-to="Gary
Gregory">Attacker-controlled exception message forges frames in
getRootCauseStackTrace output AND suppresses all real frames, beyond cosmetic
log spoofing (f025).</action>
<!-- ADD -->
<action type="add" dev="ggregory" due-to="Gary
Gregory">Add JavaVersion.JAVA_27.</action>
<action type="add" dev="ggregory" due-to="Gary
Gregory">Add SystemUtils.IS_JAVA_27.</action>
diff --git
a/src/main/java/org/apache/commons/lang3/exception/ExceptionUtils.java
b/src/main/java/org/apache/commons/lang3/exception/ExceptionUtils.java
index cbdac8f3c..064f951b9 100644
--- a/src/main/java/org/apache/commons/lang3/exception/ExceptionUtils.java
+++ b/src/main/java/org/apache/commons/lang3/exception/ExceptionUtils.java
@@ -344,6 +344,14 @@ public static String getRootCauseMessage(final Throwable
throwable) {
* exceptions separated by '[wrapped]'. Note that this is the opposite
* order to the JDK1.4 display.</p>
*
+ * <p>
+ * <strong>Note:</strong> the frames are recovered by re-parsing the text
produced by {@link Throwable#printStackTrace()}, they are not read from
+ * {@link Throwable#getStackTrace()}. A line inside an exception
<em>message</em> that mimics a stack frame (leading whitespace, then {@code "at
"},
+ * then a class/method reference with {@code '('}) is indistinguishable
from a real frame: untrusted message content can therefore inject fabricated
+ * frames into this output and cause the real frames that follow to be
dropped. Do not treat this output as forensic evidence when exception messages
+ * may contain untrusted input; read {@link Throwable#getStackTrace()} for
structured frames that cannot be forged by message content.
+ * </p>
+ *
* @param throwable The throwable to examine, may be null.
* @return An array of stack trace frames, never null.
* @since 2.0
@@ -360,6 +368,14 @@ public static String[] getRootCauseStackTrace(final
Throwable throwable) {
* its wrapping exceptions separated by '[wrapped]'. Note that this is the
opposite order to the JDK1.4 display.
* </p>
*
+ * <p>
+ * <strong>Note:</strong> the frames are recovered by re-parsing the text
produced by {@link Throwable#printStackTrace()}, they are not read from
+ * {@link Throwable#getStackTrace()}. A line inside an exception
<em>message</em> that mimics a stack frame (leading whitespace, then {@code "at
"},
+ * then a class/method reference with {@code '('}) is indistinguishable
from a real frame: untrusted message content can therefore inject fabricated
+ * frames into this output and cause the real frames that follow to be
dropped. Do not treat this output as forensic evidence when exception messages
+ * may contain untrusted input; read {@link Throwable#getStackTrace()} for
structured frames that cannot be forged by message content.
+ * </p>
+ *
* @param throwable The throwable to examine, may be null.
* @return A list of stack trace frames, never null.
* @since 3.13.0
@@ -393,8 +409,10 @@ public static List<String>
getRootCauseStackTraceList(final Throwable throwable)
* is not included. Only the trace of the specified exception is
* returned, any caused by trace is stripped.
*
- * <p>This works in most cases and will only fail if the exception
- * message contains a line that starts with: {@code "<whitespace>at"}.</p>
+ * <p>This works by re-parsing the text produced by {@link
Throwable#printStackTrace()}: a line is treated as a frame if, after leading
+ * whitespace, it starts with {@code "at "} followed by a class/method
reference and {@code '('} (see {@link #isStackFrame(String)}). It
+ * will mis-parse if the exception message contains a line of exactly that
shape: such a line is counted as a frame and the real frames
+ * that follow the remaining message lines are dropped.</p>
*
* @param throwable is any throwable.
* @return List of stack frames.
@@ -407,9 +425,7 @@ static List<String> getStackFrameList(final Throwable
throwable) {
boolean traceStarted = false;
while (frames.hasMoreTokens()) {
final String token = frames.nextToken();
- // Determine if the line starts with "<whitespace>at"
- final int at = token.indexOf("at");
- if (at != NOT_FOUND && token.substring(0, at).trim().isEmpty()) {
+ if (isStackFrame(token)) {
traceStarted = true;
list.add(token);
} else if (traceStarted) {
@@ -686,6 +702,47 @@ public static boolean isChecked(final Throwable throwable)
{
return throwable != null && !(throwable instanceof Error) &&
!(throwable instanceof RuntimeException);
}
+ /**
+ * Tests whether a line from {@link #getStackTrace(Throwable)} output
looks like a stack frame, mirroring the syntax emitted by
+ * {@link Throwable#printStackTrace()}: leading whitespace, then {@code
"at "}, then a class/method reference containing no whitespace,
+ * then {@code '('}, for example {@code "\tat
com.example.Foo.bar(Foo.java:42)"}. The reference is matched as any non-empty
run of
+ * non-whitespace characters, because {@link StackTraceElement#toString()}
never emits whitespace before the opening parenthesis: this
+ * accepts classic frames as well as class loader or module prefixes
({@code "app//"}, {@code "java.base/"}), module versions
+ * ({@code "[email protected]/"}), lambda and hidden-class names ({@code
"$$Lambda$17/0x..."}), {@code <init>}/{@code <clinit>} and
+ * JVM-language name mangling, without maintaining a character whitelist
that could reject a legitimate frame (and thereby suppress
+ * it and every frame below it).
+ *
+ * <p>This is deliberately stricter than matching any line whose first
non-whitespace characters are {@code "at"}, so that ordinary
+ * message text such as {@code " attack detected"} or {@code "at your
request"} is not mistaken for a frame; a message line crafted to
+ * match the full frame syntax is still indistinguishable from a real
frame.</p>
+ *
+ * @param token one line of printed stack trace text.
+ * @return whether the line has the syntax of a printed stack frame.
+ */
+ private static boolean isStackFrame(final String token) {
+ int i = 0;
+ final int len = token.length();
+ while (i < len && Character.isWhitespace(token.charAt(i))) {
+ i++;
+ }
+ // Frames printed by Throwable are indented: require leading
whitespace, then "at ".
+ if (i == 0 || !token.startsWith("at ", i)) {
+ return false;
+ }
+ i += 3;
+ final int paren = token.indexOf('(', i);
+ if (paren <= i) {
+ return false;
+ }
+ // StackTraceElement.toString() never emits whitespace between "at "
and '(': any whitespace there means message text.
+ for (int j = i; j < paren; j++) {
+ if (Character.isWhitespace(token.charAt(j))) {
+ return false;
+ }
+ }
+ return true;
+ }
+
/**
* Checks if a throwable represents an unchecked exception
*
@@ -715,6 +772,14 @@ public static boolean isUnchecked(final Throwable
throwable) {
* that don't have nested causes.
* </p>
*
+ * <p>
+ * <strong>Note:</strong> the frames are recovered by re-parsing the text
produced by {@link Throwable#printStackTrace()}, they are not read from
+ * {@link Throwable#getStackTrace()}. A line inside an exception
<em>message</em> that mimics a stack frame (leading whitespace, then {@code "at
"},
+ * then a class/method reference with {@code '('}) is indistinguishable
from a real frame: untrusted message content can therefore inject fabricated
+ * frames into this output and cause the real frames that follow to be
dropped. Do not treat this output as forensic evidence when exception messages
+ * may contain untrusted input; read {@link Throwable#getStackTrace()} for
structured frames that cannot be forged by message content.
+ * </p>
+ *
* @param throwable The throwable to output.
* @since 2.0
*/
@@ -736,6 +801,14 @@ public static void printRootCauseStackTrace(final
Throwable throwable) {
* <p>The method is equivalent to {@code printStackTrace} for throwables
* that don't have nested causes.</p>
*
+ * <p>
+ * <strong>Note:</strong> the frames are recovered by re-parsing the text
produced by {@link Throwable#printStackTrace()}, they are not read from
+ * {@link Throwable#getStackTrace()}. A line inside an exception
<em>message</em> that mimics a stack frame (leading whitespace, then {@code "at
"},
+ * then a class/method reference with {@code '('}) is indistinguishable
from a real frame: untrusted message content can therefore inject fabricated
+ * frames into this output and cause the real frames that follow to be
dropped. Do not treat this output as forensic evidence when exception messages
+ * may contain untrusted input; read {@link Throwable#getStackTrace()} for
structured frames that cannot be forged by message content.
+ * </p>
+ *
* @param throwable The throwable to output, may be null.
* @param printStream The stream to output to, may not be null.
* @throws NullPointerException if the printStream is {@code null}.
@@ -765,6 +838,14 @@ public static void printRootCauseStackTrace(final
Throwable throwable, final Pri
* <p>The method is equivalent to {@code printStackTrace} for throwables
* that don't have nested causes.</p>
*
+ * <p>
+ * <strong>Note:</strong> the frames are recovered by re-parsing the text
produced by {@link Throwable#printStackTrace()}, they are not read from
+ * {@link Throwable#getStackTrace()}. A line inside an exception
<em>message</em> that mimics a stack frame (leading whitespace, then {@code "at
"},
+ * then a class/method reference with {@code '('}) is indistinguishable
from a real frame: untrusted message content can therefore inject fabricated
+ * frames into this output and cause the real frames that follow to be
dropped. Do not treat this output as forensic evidence when exception messages
+ * may contain untrusted input; read {@link Throwable#getStackTrace()} for
structured frames that cannot be forged by message content.
+ * </p>
+ *
* @param throwable The throwable to output, may be null.
* @param printWriter The writer to output to, may not be null.
* @throws NullPointerException if the printWriter is {@code null}.
diff --git
a/src/test/java/org/apache/commons/lang3/exception/ExceptionUtilsTest.java
b/src/test/java/org/apache/commons/lang3/exception/ExceptionUtilsTest.java
index 35d07b21d..47626fcf6 100644
--- a/src/test/java/org/apache/commons/lang3/exception/ExceptionUtilsTest.java
+++ b/src/test/java/org/apache/commons/lang3/exception/ExceptionUtilsTest.java
@@ -26,6 +26,7 @@
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
@@ -35,6 +36,7 @@
import java.lang.reflect.Constructor;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;
@@ -391,6 +393,95 @@ void testGetRootCauseStackTraceList_Throwable() {
assertFalse(match);
}
+ /**
+ * Tests that ordinary message lines whose first non-whitespace characters
happen to be "at" are no longer mistaken for stack
+ * frames, so a multi-line untrusted message can neither inject a
non-frame-shaped line into the frame list nor suppress the
+ * real frames that follow it.
+ */
+ @Test
+ void testGetRootCauseStackTraceMessageLinesNotMistakenForFrames() {
+ final Throwable t = new IllegalArgumentException(
+ "denied" + System.lineSeparator() + " attack detected" +
System.lineSeparator() + "at your request, more text");
+ final String[] stackTrace = ExceptionUtils.getRootCauseStackTrace(t);
+ // No fabricated entries: every frame line after the header parses as
"at <ref>(...".
+ boolean sawRealFrame = false;
+ for (int i = 1; i < stackTrace.length; i++) {
+ final String element = stackTrace[i];
+ if (element.contains("attack detected") || element.contains("at
your request")) {
+ fail("message text classified as a stack frame: " + element);
+ }
+ if (element.contains(getClass().getSimpleName())) {
+ sawRealFrame = true;
+ }
+ }
+ // The real frames survive: this test method must be present in the
parsed trace.
+ assertTrue(sawRealFrame, "real frames were suppressed");
+ }
+
+ /**
+ * Tests that every frame shape {@code StackTraceElement.toString()} can
emit is accepted by the tightened frame matcher,
+ * in particular JDK 9+ module-versioned frames ({@code
mod@version/pkg.Class}), which a character whitelist without
+ * {@code '@'} would reject — silently dropping that frame and every real
frame below it. Uses a throwable that prints a
+ * fixed trace so the shapes are deterministic without constructing
module-versioned {@link StackTraceElement}s.
+ */
+ @Test
+ void testGetStackFrameListAcceptsAllRealFrameShapes() {
+ final String[] frames = {
+ "\tat com.example.Foo.bar(Foo.java:42)",
// classic
+ "\tat app//com.foo.Main.main(Main.java:10)",
// class loader prefix
+ "\tat [email protected]/com.foo.Helper.help(Helper.java:7)",
// module name @ version
+ "\tat java.base/java.lang.Thread.run(Thread.java:833)",
// module, no version
+ "\tat com.foo.Main$$Lambda$17/0x0000000800c02a48.run(Unknown
Source)", // lambda / hidden class
+ "\tat com.example.Foo.<init>(Foo.java:5)",
// constructor
+ "\tat java.base/java.lang.Object.wait(Native Method)"
// native
+ };
+ final StringBuilder text = new
StringBuilder("java.lang.RuntimeException:
boom").append(System.lineSeparator());
+ for (final String frame : frames) {
+ text.append(frame).append(System.lineSeparator());
+ }
+ final Throwable fixed = new RuntimeException("boom") {
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ public void printStackTrace(final PrintWriter writer) {
+ writer.print(text);
+ }
+ };
+ final List<String> list = ExceptionUtils.getStackFrameList(fixed);
+ assertEquals(Arrays.asList(frames), list, "a legitimate frame shape
was rejected (and frames below it dropped)");
+ }
+
+ /**
+ * Tests that message text is still rejected by the frame matcher: forged
lines lacking the no-whitespace-before-'('
+ * frame syntax must not start or extend the frame list.
+ */
+ @Test
+ void testGetStackFrameListRejectsForgedMessageLines() {
+ final String[] forged = {
+ " attack detected", // "at" not
followed by space-delimited reference
+ "at your request, more text", // no leading
whitespace
+ "\tat your request, more text", // no '(' at all
+ "\tat forged frame entry(Evil.java:1)", // whitespace
between "at " and '('
+ "\tat (Evil.java:1)" // empty reference
+ };
+ final StringBuilder text = new
StringBuilder("java.lang.RuntimeException:
boom").append(System.lineSeparator());
+ for (final String line : forged) {
+ text.append(line).append(System.lineSeparator());
+ }
+ text.append("\tat
com.example.Foo.bar(Foo.java:42)").append(System.lineSeparator());
+ final Throwable fixed = new RuntimeException("boom") {
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ public void printStackTrace(final PrintWriter writer) {
+ writer.print(text);
+ }
+ };
+ final List<String> list = ExceptionUtils.getStackFrameList(fixed);
+ assertEquals(Arrays.asList("\tat com.example.Foo.bar(Foo.java:42)"),
list,
+ "forged message text was classified as a stack frame");
+ }
+
@Test
/** getStackFrames returns empty string array when the argument is null */
void testgetStackFramesHappyPath() {
