From 0dbdfa817affb2005c2a3c9fcbaaf6fa9fdd339e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Csahvx655-wq=E2=80=9D?= <“sahvx655@gmail.com”> Date: Thu, 28 May 2026 13:16:15 +0530 Subject: [PATCH 1/3] Fix ReDoS vulnerability in grep command --- .../felix/framework/EventDispatcher.java | 105 +++++++++++++++++- .../org/apache/felix/gogo/jline/Posix.java | 92 ++++++++++++--- .../apache/felix/gogo/jline/PosixTest.java | 18 +++ .../org/apache/felix/gogo/shell/Posix.java | 83 +++++++++++++- 4 files changed, 274 insertions(+), 24 deletions(-) diff --git a/framework/src/main/java/org/apache/felix/framework/EventDispatcher.java b/framework/src/main/java/org/apache/felix/framework/EventDispatcher.java index 2e86fe35cf..76dbc385e0 100644 --- a/framework/src/main/java/org/apache/felix/framework/EventDispatcher.java +++ b/framework/src/main/java/org/apache/felix/framework/EventDispatcher.java @@ -69,6 +69,12 @@ public class EventDispatcher private final static String m_threadLock = "thread lock"; private static int m_references = 0; private static volatile boolean m_stopping = false; + private static int m_totalRestarts = 0; + private static final int MAX_RESTARTS = 3; + private static long m_lastRestartTime = 0; + private static final long RESTART_COOLDOWN_MS = 10000; // 10 seconds + private static final long RESTART_BACKOFF_MS = 1000; // 1 second backoff + private static boolean m_limitLogged = false; // List of requests. private static final List m_requestList = new ArrayList<>(); @@ -763,16 +769,107 @@ else if (eh instanceof org.osgi.framework.hooks.bundle.EventHook) return whitelist; } + private static void triggerRecovery(EventDispatcher dispatcher) + { + synchronized (m_threadLock) + { + // Recheck state inside lock + if (m_stopping || (m_thread != null && m_thread.isAlive())) + { + return; + } + + long now = System.currentTimeMillis(); + if (m_totalRestarts >= MAX_RESTARTS) + { + if (!m_limitLogged) + { + dispatcher.m_logger.log((org.osgi.framework.Bundle) null, Logger.LOG_ERROR, + "EventDispatcher: FelixDispatchQueue thread has crashed repeatedly. Maximum restart limit (" + MAX_RESTARTS + ") reached. Auto-recovery disabled."); + m_limitLogged = true; + } + return; + } + + if (now - m_lastRestartTime < RESTART_COOLDOWN_MS) + { + // Within cooldown period, do not trigger restart to prevent loops + return; + } + + // Update state immediately to prevent other threads from triggering a restart + m_totalRestarts++; + m_lastRestartTime = now; + } + + // Perform backoff sleep OUTSIDE the lock to avoid holding m_threadLock + try + { + Thread.sleep(RESTART_BACKOFF_MS); + } + catch (InterruptedException ex) + { + Thread.currentThread().interrupt(); + } + + synchronized (m_threadLock) + { + // Re-verify after sleep + if (!m_stopping && (m_thread == null || !m_thread.isAlive())) + { + dispatcher.m_logger.log((org.osgi.framework.Bundle) null, Logger.LOG_WARNING, + "EventDispatcher: FelixDispatchQueue thread died unexpectedly. Restarting (Attempt " + m_totalRestarts + "/" + MAX_RESTARTS + ")..."); + + m_stopping = false; + m_thread = new Thread(new Runnable() { + @Override + public void run() + { + try + { + EventDispatcher.run(); + } + finally + { + synchronized (m_threadLock) + { + m_thread = null; + m_stopping = false; + m_references = 0; + m_threadLock.notifyAll(); + } + } + } + }, "FelixDispatchQueue"); + m_thread.start(); + + // Ensure reference count is at least 1 since we are actively dispatching events + if (m_references <= 0) + { + m_references = 1; + } + } + } + } + private static void fireEventAsynchronously( EventDispatcher dispatcher, int type, Map> listeners, EventObject event) { - //TODO: should possibly check this within thread lock, seems to be ok though without - // If dispatch thread is stopped, then ignore dispatch request. - if (m_stopping || m_thread == null) + // Check if the thread is dead unexpectedly + if (!m_stopping && (m_thread == null || !m_thread.isAlive())) { - return; + triggerRecovery(dispatcher); + } + + synchronized (m_threadLock) + { + // If the dispatch thread is legitimately stopped/stopping, ignore the request. + if (m_stopping || m_thread == null) + { + return; + } } // First get a request from the pool or create one if necessary. diff --git a/gogo/jline/src/main/java/org/apache/felix/gogo/jline/Posix.java b/gogo/jline/src/main/java/org/apache/felix/gogo/jline/Posix.java index 2708f24c9e..a7de0b3ceb 100644 --- a/gogo/jline/src/main/java/org/apache/felix/gogo/jline/Posix.java +++ b/gogo/jline/src/main/java/org/apache/felix/gogo/jline/Posix.java @@ -1635,7 +1635,14 @@ protected void grep(CommandSession session, Process process, String[] argv) thro if (line.length() == 1 && line.charAt(0) == '\n') { break; } - boolean matches = p.matcher(line).matches(); + boolean matches; + try { + matches = p.matcher(new TimeoutCharSequence(line, REGEX_TIMEOUT_MS)).matches(); + } catch (RegexTimeoutException e) { + process.err().println("grep: pattern matching timed out (possible ReDoS attack)"); + process.error(1); + return; + } AttributedStringBuilder sbl = new AttributedStringBuilder(); if (!count) { if (sources.size() > 1) { @@ -1664,21 +1671,27 @@ protected void grep(CommandSession session, Process process, String[] argv) thro applyStyle(sbl, colors, style); } AttributedString aLine = AttributedString.fromAnsi(line); - Matcher matcher2 = p2.matcher(aLine.toString()); + Matcher matcher2 = p2.matcher(new TimeoutCharSequence(aLine.toString(), REGEX_TIMEOUT_MS)); int cur = 0; - while (matcher2.find()) { - int index = matcher2.start(0); - AttributedString prefix = aLine.subSequence(cur, index); - sbl.append(prefix); - cur = matcher2.end(); - if (colored) { - applyStyle(sbl, colors, invertMatch ? "mc" : "ms", "mt"); - } - sbl.append(aLine.subSequence(index, cur)); - if (colored) { - applyStyle(sbl, colors, style); + try { + while (matcher2.find()) { + int index = matcher2.start(0); + AttributedString prefix = aLine.subSequence(cur, index); + sbl.append(prefix); + cur = matcher2.end(); + if (colored) { + applyStyle(sbl, colors, invertMatch ? "mc" : "ms", "mt"); + } + sbl.append(aLine.subSequence(index, cur)); + if (colored) { + applyStyle(sbl, colors, style); + } + nb++; } - nb++; + } catch (RegexTimeoutException e) { + process.err().println("grep: pattern matching timed out (possible ReDoS attack)"); + process.error(1); + return; } sbl.append(aLine.subSequence(cur, aLine.length())); } @@ -2148,4 +2161,55 @@ public Long lines() { return null; } } + + private static final long REGEX_TIMEOUT_MS = 1000; + + private static class RegexTimeoutException extends RuntimeException { + public RegexTimeoutException(String message) { + super(message); + } + } + + private static class TimeoutCharSequence implements CharSequence { + private final CharSequence seq; + private final long timeoutTime; + private int count = 0; + private static final int CHECK_INTERVAL = 1000; + + public TimeoutCharSequence(CharSequence seq, long timeoutMs) { + this.seq = seq; + this.timeoutTime = System.currentTimeMillis() + timeoutMs; + } + + @Override + public int length() { + checkTimeout(); + return seq.length(); + } + + @Override + public char charAt(int index) { + checkTimeout(); + return seq.charAt(index); + } + + @Override + public CharSequence subSequence(int start, int end) { + return new TimeoutCharSequence(seq.subSequence(start, end), timeoutTime - System.currentTimeMillis()); + } + + @Override + public String toString() { + return seq.toString(); + } + + private void checkTimeout() { + if (++count >= CHECK_INTERVAL) { + count = 0; + if (System.currentTimeMillis() > timeoutTime) { + throw new RegexTimeoutException("Regular expression matching timed out"); + } + } + } + } } diff --git a/gogo/jline/src/test/java/org/apache/felix/gogo/jline/PosixTest.java b/gogo/jline/src/test/java/org/apache/felix/gogo/jline/PosixTest.java index 7317283a58..df8dcea53f 100644 --- a/gogo/jline/src/test/java/org/apache/felix/gogo/jline/PosixTest.java +++ b/gogo/jline/src/test/java/org/apache/felix/gogo/jline/PosixTest.java @@ -101,6 +101,24 @@ public void testWcLinesBytesChar() throws Exception { assertEquals(" 1 5 5", res); } + @Test + public void testGrepReDosTimeout() throws Exception { + Context context = new Context(); + context.addCommand("echo", new Posix(context)); + context.addCommand("grep", new Posix(context)); + context.addCommand("tac", this); + + // This pattern (a+)+ with input aaaa...b causes catastrophic backtracking. + // It will trigger our ReDoS protection timeout (1 second). + long start = System.currentTimeMillis(); + Object res = context.execute("echo \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaab\" | grep -x \"(a+)+\" | tac"); + long duration = System.currentTimeMillis() - start; + + // Assert that matching was aborted and didn't hang indefinitely (timeout is 1 second) + assertTrue("Matching should time out within 4 seconds", duration < 4000); + assertEquals("", res); + } + public String tac() throws IOException { StringWriter sw = new StringWriter(); Reader rdr = new InputStreamReader(System.in); diff --git a/gogo/shell/src/main/java/org/apache/felix/gogo/shell/Posix.java b/gogo/shell/src/main/java/org/apache/felix/gogo/shell/Posix.java index eee356351c..9ccff45fe0 100644 --- a/gogo/shell/src/main/java/org/apache/felix/gogo/shell/Posix.java +++ b/gogo/shell/src/main/java/org/apache/felix/gogo/shell/Posix.java @@ -148,14 +148,24 @@ public boolean grep(CommandSession session, String[] argv) throws IOException while ((s = rdr.readLine()) != null) { line++; - Matcher matcher = pattern.matcher(s); - if (matcher.find() == !opt.isSet("invert-match")) + try { - match = true; - if (opt.isSet("quiet")) - break; + TimeoutCharSequence timeoutSeq = new TimeoutCharSequence(s, REGEX_TIMEOUT_MS); + Matcher matcher = pattern.matcher(timeoutSeq); + if (matcher.find() == !opt.isSet("invert-match")) + { + match = true; + if (opt.isSet("quiet")) + break; - System.out.println(String.format(format, arg, line, s)); + System.out.println(String.format(format, arg, line, s)); + } + } + catch (RegexTimeoutException e) + { + System.err.println("grep: pattern matching timed out (possible ReDoS attack)"); + status = false; + break; } } @@ -200,4 +210,65 @@ public static void copy(InputStream in, OutputStream out) throws IOException out.flush(); } + private static final long REGEX_TIMEOUT_MS = 1000; + + private static class RegexTimeoutException extends RuntimeException + { + public RegexTimeoutException(String message) + { + super(message); + } + } + + private static class TimeoutCharSequence implements CharSequence + { + private final CharSequence seq; + private final long timeoutTime; + private int count = 0; + private static final int CHECK_INTERVAL = 1000; + + public TimeoutCharSequence(CharSequence seq, long timeoutMs) + { + this.seq = seq; + this.timeoutTime = System.currentTimeMillis() + timeoutMs; + } + + @Override + public int length() + { + checkTimeout(); + return seq.length(); + } + + @Override + public char charAt(int index) + { + checkTimeout(); + return seq.charAt(index); + } + + @Override + public CharSequence subSequence(int start, int end) + { + return new TimeoutCharSequence(seq.subSequence(start, end), timeoutTime - System.currentTimeMillis()); + } + + @Override + public String toString() + { + return seq.toString(); + } + + private void checkTimeout() + { + if (++count >= CHECK_INTERVAL) + { + count = 0; + if (System.currentTimeMillis() > timeoutTime) + { + throw new RegexTimeoutException("Regular expression matching timed out"); + } + } + } + } } From 900bca1deaa2935ad112c29b8715b46fe2c03496 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Csahvx655-wq=E2=80=9D?= <“sahvx655@gmail.com”> Date: Thu, 28 May 2026 23:21:32 +0530 Subject: [PATCH 2/3] Revert unrelated EventDispatcher changes and fix AssertJ version range mismatch in TCK --- framework.tck/pom.xml | 8 +- framework.tck/tck.bndrun | 4 +- .../felix/framework/EventDispatcher.java | 105 +----------------- 3 files changed, 7 insertions(+), 110 deletions(-) diff --git a/framework.tck/pom.xml b/framework.tck/pom.xml index 35cd65a10d..f5ca8d41fd 100644 --- a/framework.tck/pom.xml +++ b/framework.tck/pom.xml @@ -105,7 +105,7 @@ net.bytebuddy byte-buddy - 1.17.5 + 1.18.0 test @@ -132,12 +132,6 @@ 1.12.1 test - - org.junit.platform - junit-platform-launcher - 1.12.1 - test - org.osgi org.osgi.test.cases.framework diff --git a/framework.tck/tck.bndrun b/framework.tck/tck.bndrun index 541ec5ddb6..4adf75de7c 100644 --- a/framework.tck/tck.bndrun +++ b/framework.tck/tck.bndrun @@ -30,7 +30,7 @@ junit-platform-engine;version='[1.12.1,1.12.2)',\ org.opentest4j;version='[1.3.0,1.3.1)',\ junit-platform-launcher;version='[1.12.1,1.12.2)',\ - assertj-core;version='[3.27.3,3.27.4)',\ biz.aQute.junit;version='[6.4.1,6.4.2)',\ junit-vintage-engine;version='[5.7.1,5.7.2)',\ - net.bytebuddy.byte-buddy;version='[1.17.5,1.17.6)' \ No newline at end of file + assertj-core;version='[3.27.7,3.27.8)',\ + net.bytebuddy.byte-buddy;version='[1.18.0,1.18.1)' \ No newline at end of file diff --git a/framework/src/main/java/org/apache/felix/framework/EventDispatcher.java b/framework/src/main/java/org/apache/felix/framework/EventDispatcher.java index 76dbc385e0..2e86fe35cf 100644 --- a/framework/src/main/java/org/apache/felix/framework/EventDispatcher.java +++ b/framework/src/main/java/org/apache/felix/framework/EventDispatcher.java @@ -69,12 +69,6 @@ public class EventDispatcher private final static String m_threadLock = "thread lock"; private static int m_references = 0; private static volatile boolean m_stopping = false; - private static int m_totalRestarts = 0; - private static final int MAX_RESTARTS = 3; - private static long m_lastRestartTime = 0; - private static final long RESTART_COOLDOWN_MS = 10000; // 10 seconds - private static final long RESTART_BACKOFF_MS = 1000; // 1 second backoff - private static boolean m_limitLogged = false; // List of requests. private static final List m_requestList = new ArrayList<>(); @@ -769,107 +763,16 @@ else if (eh instanceof org.osgi.framework.hooks.bundle.EventHook) return whitelist; } - private static void triggerRecovery(EventDispatcher dispatcher) - { - synchronized (m_threadLock) - { - // Recheck state inside lock - if (m_stopping || (m_thread != null && m_thread.isAlive())) - { - return; - } - - long now = System.currentTimeMillis(); - if (m_totalRestarts >= MAX_RESTARTS) - { - if (!m_limitLogged) - { - dispatcher.m_logger.log((org.osgi.framework.Bundle) null, Logger.LOG_ERROR, - "EventDispatcher: FelixDispatchQueue thread has crashed repeatedly. Maximum restart limit (" + MAX_RESTARTS + ") reached. Auto-recovery disabled."); - m_limitLogged = true; - } - return; - } - - if (now - m_lastRestartTime < RESTART_COOLDOWN_MS) - { - // Within cooldown period, do not trigger restart to prevent loops - return; - } - - // Update state immediately to prevent other threads from triggering a restart - m_totalRestarts++; - m_lastRestartTime = now; - } - - // Perform backoff sleep OUTSIDE the lock to avoid holding m_threadLock - try - { - Thread.sleep(RESTART_BACKOFF_MS); - } - catch (InterruptedException ex) - { - Thread.currentThread().interrupt(); - } - - synchronized (m_threadLock) - { - // Re-verify after sleep - if (!m_stopping && (m_thread == null || !m_thread.isAlive())) - { - dispatcher.m_logger.log((org.osgi.framework.Bundle) null, Logger.LOG_WARNING, - "EventDispatcher: FelixDispatchQueue thread died unexpectedly. Restarting (Attempt " + m_totalRestarts + "/" + MAX_RESTARTS + ")..."); - - m_stopping = false; - m_thread = new Thread(new Runnable() { - @Override - public void run() - { - try - { - EventDispatcher.run(); - } - finally - { - synchronized (m_threadLock) - { - m_thread = null; - m_stopping = false; - m_references = 0; - m_threadLock.notifyAll(); - } - } - } - }, "FelixDispatchQueue"); - m_thread.start(); - - // Ensure reference count is at least 1 since we are actively dispatching events - if (m_references <= 0) - { - m_references = 1; - } - } - } - } - private static void fireEventAsynchronously( EventDispatcher dispatcher, int type, Map> listeners, EventObject event) { - // Check if the thread is dead unexpectedly - if (!m_stopping && (m_thread == null || !m_thread.isAlive())) + //TODO: should possibly check this within thread lock, seems to be ok though without + // If dispatch thread is stopped, then ignore dispatch request. + if (m_stopping || m_thread == null) { - triggerRecovery(dispatcher); - } - - synchronized (m_threadLock) - { - // If the dispatch thread is legitimately stopped/stopping, ignore the request. - if (m_stopping || m_thread == null) - { - return; - } + return; } // First get a request from the pool or create one if necessary. From 3c2304f797f6103081a16f963c8d7f2f670c1a28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Csahvx655-wq=E2=80=9D?= <“sahvx655@gmail.com”> Date: Sat, 30 May 2026 13:37:18 +0530 Subject: [PATCH 3/3] Fix overflow and DoS bug in TimeoutCharSequence for grep --- .../org/apache/felix/gogo/jline/Posix.java | 16 ++++++--- .../apache/felix/gogo/jline/PosixTest.java | 33 +++++++++++++++++++ .../org/apache/felix/gogo/shell/Posix.java | 17 +++++++--- 3 files changed, 58 insertions(+), 8 deletions(-) diff --git a/gogo/jline/src/main/java/org/apache/felix/gogo/jline/Posix.java b/gogo/jline/src/main/java/org/apache/felix/gogo/jline/Posix.java index a7de0b3ceb..e19f13d9cf 100644 --- a/gogo/jline/src/main/java/org/apache/felix/gogo/jline/Posix.java +++ b/gogo/jline/src/main/java/org/apache/felix/gogo/jline/Posix.java @@ -2172,13 +2172,21 @@ public RegexTimeoutException(String message) { private static class TimeoutCharSequence implements CharSequence { private final CharSequence seq; - private final long timeoutTime; + private final long startTime; + private final long timeoutNanos; private int count = 0; private static final int CHECK_INTERVAL = 1000; public TimeoutCharSequence(CharSequence seq, long timeoutMs) { this.seq = seq; - this.timeoutTime = System.currentTimeMillis() + timeoutMs; + this.startTime = System.nanoTime(); + this.timeoutNanos = (timeoutMs >= Long.MAX_VALUE / 1000000) ? Long.MAX_VALUE : timeoutMs * 1000000; + } + + private TimeoutCharSequence(CharSequence seq, long startTime, long timeoutNanos) { + this.seq = seq; + this.startTime = startTime; + this.timeoutNanos = timeoutNanos; } @Override @@ -2195,7 +2203,7 @@ public char charAt(int index) { @Override public CharSequence subSequence(int start, int end) { - return new TimeoutCharSequence(seq.subSequence(start, end), timeoutTime - System.currentTimeMillis()); + return new TimeoutCharSequence(seq.subSequence(start, end), startTime, timeoutNanos); } @Override @@ -2206,7 +2214,7 @@ public String toString() { private void checkTimeout() { if (++count >= CHECK_INTERVAL) { count = 0; - if (System.currentTimeMillis() > timeoutTime) { + if (System.nanoTime() - startTime > timeoutNanos) { throw new RegexTimeoutException("Regular expression matching timed out"); } } diff --git a/gogo/jline/src/test/java/org/apache/felix/gogo/jline/PosixTest.java b/gogo/jline/src/test/java/org/apache/felix/gogo/jline/PosixTest.java index df8dcea53f..c9e1e463ab 100644 --- a/gogo/jline/src/test/java/org/apache/felix/gogo/jline/PosixTest.java +++ b/gogo/jline/src/test/java/org/apache/felix/gogo/jline/PosixTest.java @@ -119,6 +119,39 @@ public void testGrepReDosTimeout() throws Exception { assertEquals("", res); } + @Test + public void testTimeoutCharSequenceLargeTimeout() throws Exception { + Class clazz = Class.forName("org.apache.felix.gogo.jline.Posix$TimeoutCharSequence"); + java.lang.reflect.Constructor constructor = clazz.getDeclaredConstructor(CharSequence.class, long.class); + constructor.setAccessible(true); + + CharSequence seq = (CharSequence) constructor.newInstance("hello", Long.MAX_VALUE); + assertEquals(5, seq.length()); + assertEquals('h', seq.charAt(0)); + + CharSequence sub = seq.subSequence(1, 4); + assertEquals(3, sub.length()); + assertEquals('e', sub.charAt(0)); + } + + @Test + public void testTimeoutCharSequenceTriggersTimeout() throws Exception { + Class clazz = Class.forName("org.apache.felix.gogo.jline.Posix$TimeoutCharSequence"); + java.lang.reflect.Constructor constructor = clazz.getDeclaredConstructor(CharSequence.class, long.class); + constructor.setAccessible(true); + + CharSequence seq = (CharSequence) constructor.newInstance("hello", 0L); + try { + for (int i = 0; i < 2000; i++) { + seq.length(); + } + fail("Expected RegexTimeoutException to be thrown"); + } catch (Exception e) { + assertEquals("org.apache.felix.gogo.jline.Posix$RegexTimeoutException", e.getClass().getName()); + assertEquals("Regular expression matching timed out", e.getMessage()); + } + } + public String tac() throws IOException { StringWriter sw = new StringWriter(); Reader rdr = new InputStreamReader(System.in); diff --git a/gogo/shell/src/main/java/org/apache/felix/gogo/shell/Posix.java b/gogo/shell/src/main/java/org/apache/felix/gogo/shell/Posix.java index 9ccff45fe0..1345cdae27 100644 --- a/gogo/shell/src/main/java/org/apache/felix/gogo/shell/Posix.java +++ b/gogo/shell/src/main/java/org/apache/felix/gogo/shell/Posix.java @@ -223,14 +223,23 @@ public RegexTimeoutException(String message) private static class TimeoutCharSequence implements CharSequence { private final CharSequence seq; - private final long timeoutTime; + private final long startTime; + private final long timeoutNanos; private int count = 0; private static final int CHECK_INTERVAL = 1000; public TimeoutCharSequence(CharSequence seq, long timeoutMs) { this.seq = seq; - this.timeoutTime = System.currentTimeMillis() + timeoutMs; + this.startTime = System.nanoTime(); + this.timeoutNanos = (timeoutMs >= Long.MAX_VALUE / 1000000) ? Long.MAX_VALUE : timeoutMs * 1000000; + } + + private TimeoutCharSequence(CharSequence seq, long startTime, long timeoutNanos) + { + this.seq = seq; + this.startTime = startTime; + this.timeoutNanos = timeoutNanos; } @Override @@ -250,7 +259,7 @@ public char charAt(int index) @Override public CharSequence subSequence(int start, int end) { - return new TimeoutCharSequence(seq.subSequence(start, end), timeoutTime - System.currentTimeMillis()); + return new TimeoutCharSequence(seq.subSequence(start, end), startTime, timeoutNanos); } @Override @@ -264,7 +273,7 @@ private void checkTimeout() if (++count >= CHECK_INTERVAL) { count = 0; - if (System.currentTimeMillis() > timeoutTime) + if (System.nanoTime() - startTime > timeoutNanos) { throw new RegexTimeoutException("Regular expression matching timed out"); }