This is an automated email from the ASF dual-hosted git repository.
chrisdutz pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/plc4x.git
The following commit(s) were added to refs/heads/develop by this push:
new 5a4d5bdb4c fix: Addessed various issues regarding to parsing of
potentially corrupt or mallicious responses which could cause OOM errors.
5a4d5bdb4c is described below
commit 5a4d5bdb4c1b0e9356eff00af198bdb6aa960f26
Author: Christofer Dutz <[email protected]>
AuthorDate: Fri Jul 10 14:01:04 2026 +0200
fix: Addessed various issues regarding to parsing of potentially corrupt or
mallicious responses which could cause OOM errors.
---
.../plc4x/java/ads/resolution/ValueDecoder.java | 19 ++++++++++++++++++-
.../canopen/conversation/SDOUploadConversation.java | 21 ++++++++++++++++++++-
.../apache/plc4x/java/opcua/OpcuaConnection.java | 9 +++++++++
.../plc4x/java/opcua/context/Conversation.java | 18 ++++++++++++++++++
.../java/opcua/protocol/chunk/ChunkStorage.java | 7 +++++++
.../opcua/protocol/chunk/MemoryChunkStorage.java | 5 +++++
.../spi/fields/fields/reader/FieldReaderArray.java | 14 +++++++++++++-
7 files changed, 90 insertions(+), 3 deletions(-)
diff --git
a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/resolution/ValueDecoder.java
b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/resolution/ValueDecoder.java
index 8931c8619a..10bda422de 100644
---
a/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/resolution/ValueDecoder.java
+++
b/plc4j/drivers/ads/src/main/java/org/apache/plc4x/java/ads/resolution/ValueDecoder.java
@@ -48,6 +48,14 @@ import java.util.Map;
*/
public final class ValueDecoder {
+ /**
+ * Upper bound for the initial capacity of a list whose element count
comes from the
+ * (server-supplied) data type table. Prevents a bogus,
attacker-controlled array dimension
+ * from triggering an eager multi-GB allocation before any element is
actually read. The list
+ * still grows past this if that many elements really are present on the
wire.
+ */
+ private static final int MAX_INITIAL_CAPACITY = 1024;
+
private final Map<String, AdsDataTypeTableEntry> dataTypeTable;
public ValueDecoder(Map<String, AdsDataTypeTableEntry> dataTypeTable) {
@@ -98,7 +106,16 @@ public final class ValueDecoder {
AdsDataTypeArrayInfo cur = dims.get(0);
List<AdsDataTypeArrayInfo> rest = dims.subList(1, dims.size());
long count = cur.getNumElements();
- List<PlcValue> elements = new ArrayList<>((int) count);
+ // numElements is a wire-supplied unsigned 32-bit value (up to
0xFFFFFFFF). Reject counts
+ // that don't fit a positive int before the (int) cast - otherwise a
large value either
+ // overflows to a negative capacity (NegativeArraySizeException) or
forces an eager
+ // multi-GB allocation (OOM / DoS). The list itself is not pre-sized
to the untrusted
+ // count: it grows as elements are actually decoded, and
dataReader.read() throws once the
+ // buffer is exhausted.
+ if (count > Integer.MAX_VALUE) {
+ throw new BufferException("Array count of " + count + " exceeds
the maximum allowed count of " + Integer.MAX_VALUE);
+ }
+ List<PlcValue> elements = new ArrayList<>((int) Math.min(count,
MAX_INITIAL_CAPACITY));
for (long i = 0; i < count; i++) {
elements.add(decodePartialArray(rb, arrayDataType, rest));
}
diff --git
a/plc4j/drivers/canopen/src/main/java/org/apache/plc4x/java/canopen/conversation/SDOUploadConversation.java
b/plc4j/drivers/canopen/src/main/java/org/apache/plc4x/java/canopen/conversation/SDOUploadConversation.java
index 9fc4fdf8fe..e9ce200666 100644
---
a/plc4j/drivers/canopen/src/main/java/org/apache/plc4x/java/canopen/conversation/SDOUploadConversation.java
+++
b/plc4j/drivers/canopen/src/main/java/org/apache/plc4x/java/canopen/conversation/SDOUploadConversation.java
@@ -57,6 +57,15 @@ public class SDOUploadConversation {
private static final Logger LOGGER =
LoggerFactory.getLogger(SDOUploadConversation.class);
+ /**
+ * Upper bound for the buffer eagerly allocated for a segmented SDO
upload. The "total bytes"
+ * figure comes from the remote node's initiate-upload response (an
unsigned 32-bit wire
+ * field), so a bogus value could otherwise trigger a multi-GB allocation
before a single
+ * segment is read (OOM / DoS). SDO uploads carry object-dictionary values
fetched in ≤7-byte
+ * segments, so 16 MiB is already an extremely generous ceiling for any
legitimate transfer.
+ */
+ private static final long MAX_SEGMENTED_UPLOAD_SIZE = 16L * 1024 * 1024;
+
private final CANConversation conversation;
private final int nodeId;
private final int answerNodeId;
@@ -118,7 +127,17 @@ public class SDOUploadConversation {
completeWithValue(receiver, payload.getData(),
payload.getData().length);
} else if (answer.getPayload() instanceof
SDOInitiateSegmentedUploadResponse) {
SDOInitiateSegmentedUploadResponse segment =
(SDOInitiateSegmentedUploadResponse) answer.getPayload();
- int totalSize = (int) segment.getBytes();
+ long announcedSize = segment.getBytes();
+ // Don't eagerly allocate a buffer sized to the untrusted,
node-announced total: a bogus
+ // value (e.g. 0x40000000) would trigger a huge allocation before
any segment is read,
+ // and 0xFFFFFFFF would even cast to a negative array size. Reject
implausible sizes up
+ // front; the buffer is still filled incrementally from the
segments actually received.
+ if (announcedSize < 0 || announcedSize >
MAX_SEGMENTED_UPLOAD_SIZE) {
+ receiver.completeExceptionally(new PlcException(
+ "SDO segmented upload announced an implausible size of " +
announcedSize + " bytes"));
+ return;
+ }
+ int totalSize = (int) announcedSize;
fetch(receiver, new byte[totalSize], 0, totalSize, false);
} else {
receiver.completeExceptionally(new PlcException("Unsupported SDO
upload kind."));
diff --git
a/plc4j/drivers/opcua/src/main/java/org/apache/plc4x/java/opcua/OpcuaConnection.java
b/plc4j/drivers/opcua/src/main/java/org/apache/plc4x/java/opcua/OpcuaConnection.java
index bf4adc2f43..b24c3dbacf 100644
---
a/plc4j/drivers/opcua/src/main/java/org/apache/plc4x/java/opcua/OpcuaConnection.java
+++
b/plc4j/drivers/opcua/src/main/java/org/apache/plc4x/java/opcua/OpcuaConnection.java
@@ -617,6 +617,15 @@ public class OpcuaConnection extends
ConnectionBase<OpcuaConfiguration> implemen
return new PlcList(flatValues);
}
int currentDim = dimensions.getFirst();
+ // The array dimensions are raw signed int32 values taken straight off
the wire, independent
+ // of how many values were actually sent. Only partition when the
declared dimension is
+ // consistent with the materialized value count: this bounds the work
to real received data
+ // and avoids both a division by zero (currentDim == 0) and an eager
multi-GB allocation /
+ // loop for an attacker-supplied dimension (e.g. 0x7fffffff).
Otherwise fall back to a flat
+ // list, matching the behaviour when no dimensions are declared.
+ if (currentDim <= 0 || currentDim > flatValues.size() ||
flatValues.size() % currentDim != 0) {
+ return new PlcList(flatValues);
+ }
List<Integer> remainingDims = dimensions.subList(1, dimensions.size());
int chunkSize = flatValues.size() / currentDim;
List<PlcValue> result = new ArrayList<>(currentDim);
diff --git
a/plc4j/drivers/opcua/src/main/java/org/apache/plc4x/java/opcua/context/Conversation.java
b/plc4j/drivers/opcua/src/main/java/org/apache/plc4x/java/opcua/context/Conversation.java
index 048fba6c4c..492880d48d 100644
---
a/plc4j/drivers/opcua/src/main/java/org/apache/plc4x/java/opcua/context/Conversation.java
+++
b/plc4j/drivers/opcua/src/main/java/org/apache/plc4x/java/opcua/context/Conversation.java
@@ -503,6 +503,24 @@ public class Conversation implements SecureChannelState {
}
storage.append(((BinaryPayload) data).getPayload());
+ // Enforce the negotiated receive limits while accumulating chunks.
Without this a malicious
+ // or faulty server could stream an unbounded number of CONTINUE
chunks (never sending a
+ // FINAL), growing the in-memory buffer without bound until the client
runs out of memory.
+ // A limit of 0 means "no limit" per the OPC UA spec, so only enforce
positive limits.
+ if (limits != null) {
+ long maxChunkCount = limits.getMaxChunkCount();
+ if (maxChunkCount > 0 && storage.count() > maxChunkCount) {
+ storage.reset();
+ throw new IllegalStateException("Received message exceeds the
negotiated maximum chunk count of " + maxChunkCount);
+ }
+ long maxMessageSize = limits.getMaxMessageSize();
+ if (maxMessageSize > 0 && storage.size() > maxMessageSize) {
+ storage.reset();
+ throw new IllegalStateException("Received message size " +
storage.size()
+ + " exceeds the negotiated maximum message size of " +
maxMessageSize + " bytes");
+ }
+ }
+
return FINAL.equals(chunkType);
}
diff --git
a/plc4j/drivers/opcua/src/main/java/org/apache/plc4x/java/opcua/protocol/chunk/ChunkStorage.java
b/plc4j/drivers/opcua/src/main/java/org/apache/plc4x/java/opcua/protocol/chunk/ChunkStorage.java
index a34e50fc25..e7e793ee28 100644
---
a/plc4j/drivers/opcua/src/main/java/org/apache/plc4x/java/opcua/protocol/chunk/ChunkStorage.java
+++
b/plc4j/drivers/opcua/src/main/java/org/apache/plc4x/java/opcua/protocol/chunk/ChunkStorage.java
@@ -35,6 +35,13 @@ public interface ChunkStorage {
*/
long size();
+ /**
+ * Gets the number of chunks accumulated so far.
+ *
+ * @return Number of appended chunks.
+ */
+ long count();
+
/**
* Retrieves final result from segmented payload.
*
diff --git
a/plc4j/drivers/opcua/src/main/java/org/apache/plc4x/java/opcua/protocol/chunk/MemoryChunkStorage.java
b/plc4j/drivers/opcua/src/main/java/org/apache/plc4x/java/opcua/protocol/chunk/MemoryChunkStorage.java
index d5b36feb82..a95d268342 100644
---
a/plc4j/drivers/opcua/src/main/java/org/apache/plc4x/java/opcua/protocol/chunk/MemoryChunkStorage.java
+++
b/plc4j/drivers/opcua/src/main/java/org/apache/plc4x/java/opcua/protocol/chunk/MemoryChunkStorage.java
@@ -38,6 +38,11 @@ public class MemoryChunkStorage implements ChunkStorage {
return size;
}
+ @Override
+ public long count() {
+ return chunks.size();
+ }
+
@Override
public byte[] get() {
Optional<byte[]> collect = chunks.stream().reduce((b1, b2) -> {
diff --git
a/plc4j/spi/fields/src/main/java/org/apache/plc4x/java/spi/fields/fields/reader/FieldReaderArray.java
b/plc4j/spi/fields/src/main/java/org/apache/plc4x/java/spi/fields/fields/reader/FieldReaderArray.java
index 0a2ba56f1e..9f454bb67f 100644
---
a/plc4j/spi/fields/src/main/java/org/apache/plc4x/java/spi/fields/fields/reader/FieldReaderArray.java
+++
b/plc4j/spi/fields/src/main/java/org/apache/plc4x/java/spi/fields/fields/reader/FieldReaderArray.java
@@ -35,6 +35,14 @@ public class FieldReaderArray<T> implements FieldCommons {
private static final Logger LOGGER =
LoggerFactory.getLogger(FieldReaderArray.class);
+ /**
+ * Upper bound for the initial capacity of an array whose element count
comes from the wire.
+ * Prevents a bogus, attacker-controlled count from triggering an eager
multi-GB allocation
+ * before any element is actually read. The list still grows past this if
that many elements
+ * really are present on the wire.
+ */
+ private static final int MAX_INITIAL_CAPACITY = 1024;
+
public List<T> readArrayFieldCount(DataReader<T> dataReader, long count,
WithOption... options) throws BufferException {
LOGGER.debug("reading field {}. Count: {}", getName(options), count);
if (count > Integer.MAX_VALUE) {
@@ -47,7 +55,11 @@ public class FieldReaderArray<T> implements FieldCommons {
//readerArgs = ArrayUtils.add(readerArgs,
WithReaderWriterArgs.WithRenderAsList(true));
dataReader.pushContext(options);
int itemCount = Math.max(0, (int) count);
- List<T> result = new ArrayList<>(itemCount);
+ // Don't eagerly pre-size the backing array to an untrusted,
wire-supplied count: a bogus
+ // count (e.g. 0x7fffffff) would trigger a multi-GB allocation before
a single element is
+ // read (OOM / DoS). Cap the initial capacity to a modest bound and
let the list grow as
+ // elements are actually decoded - once the buffer is exhausted,
dataReader.read() throws.
+ List<T> result = new ArrayList<>(Math.min(itemCount,
MAX_INITIAL_CAPACITY));
for (int curItem = 0; curItem < itemCount; curItem++) {
// Make some variables available that would be otherwise
challenging to forward.
ThreadLocalHelper.curItemThreadLocal.set(curItem);
