You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@plc4x.apache.org by cd...@apache.org on 2026/07/10 12:01:14 UTC
(plc4x) branch develop updated: fix: Addessed various issues regarding to parsing of potentially corrupt or mallicious responses which could cause OOM errors.
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 <ch...@c-ware.de>
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);