This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch geoapi-4.0 in repository https://gitbox.apache.org/repos/asf/sis.git
commit 44c63d5f25cb7b2410c47b9c9135baa14eefd07e Author: Martin Desruisseaux <[email protected]> AuthorDate: Sat Sep 12 20:56:56 2026 +0900 Add a safety against array of unreasonable length in netCDF and TIFF files. --- .../apache/sis/storage/geotiff/DeferredEntry.java | 29 ++++++++++++++++++++-- .../org/apache/sis/storage/geotiff/Reader.java | 11 +++++--- .../sis/storage/netcdf/classic/ChannelDecoder.java | 27 ++++++++++++++------ .../main/org/apache/sis/io/stream/ChannelData.java | 11 ++++++-- .../org/apache/sis/io/stream/ChannelDataInput.java | 18 ++++++++++++++ 5 files changed, 82 insertions(+), 14 deletions(-) diff --git a/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/DeferredEntry.java b/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/DeferredEntry.java index b6f719b76f..c5309d9c3b 100644 --- a/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/DeferredEntry.java +++ b/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/DeferredEntry.java @@ -16,7 +16,10 @@ */ package org.apache.sis.storage.geotiff; +import org.apache.sis.storage.DataStoreContentException; +import org.apache.sis.storage.geotiff.base.Tags; import org.apache.sis.storage.geotiff.reader.Type; +import org.apache.sis.util.resources.Errors; /** @@ -38,12 +41,13 @@ final class DeferredEntry implements Comparable<DeferredEntry> { /** * The GeoTIFF type of the value to read. */ - final Type type; + private final Type type; /** * The number of values to read. + * This value come from a 32-bits unsigned integer. Therefore, it should never be negative. */ - final long count; + private final long count; /** * Offset from beginning of TIFF file where the values are stored. @@ -71,4 +75,25 @@ final class DeferredEntry implements Comparable<DeferredEntry> { public int compareTo(final DeferredEntry other) { return Long.signum(offset - other.offset); } + + /** + * Ensures that the number of elements to read is not too large. + * + * @param remaining number of bytes remaining in the stream to read. + */ + final void ensureReasonableCount(final long remaining) throws DataStoreContentException { + if (count * type.size > remaining) { + throw new DataStoreContentException(owner.reader.errors().getString(Errors.Keys.ExcessiveListSize_2, Tags.name(tag), count)); + } + } + + /** + * Adds the value read from the current position in the given stream for this entry. + * + * @return {@code null} on success, or the unrecognized value otherwise. + * @throws Exception if an error occurred while reading the entry. + */ + final Object addDeferredEntry() throws Exception { + return owner.addEntry(tag, type, count); + } } diff --git a/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/Reader.java b/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/Reader.java index 38fb035626..472f8ff5df 100644 --- a/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/Reader.java +++ b/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/Reader.java @@ -367,13 +367,18 @@ final class Reader extends IOBase { if (stopAfter.owner == dir) break; } } + final long length = input.length(); for (final Iterator<DeferredEntry> it = deferredEntries.iterator(); it.hasNext();) { final DeferredEntry entry = it.next(); - if (entry.owner == dir || (entry.offset >= ignoreBefore && entry.offset <= ignoreAfter)) { - input.seek(entry.offset); + final long offset = entry.offset; + if (entry.owner == dir || (offset >= ignoreBefore && offset <= ignoreAfter)) { + if (length >= 0) { + entry.ensureReasonableCount(length - offset); + } + input.seek(offset); Object error; try { - error = entry.owner.addEntry(entry.tag, entry.type, entry.count); + error = entry.addDeferredEntry(); } catch (IOException | DataStoreException e) { throw e; } catch (Exception e) { diff --git a/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/classic/ChannelDecoder.java b/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/classic/ChannelDecoder.java index 8e9536d59b..0eaba588bf 100644 --- a/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/classic/ChannelDecoder.java +++ b/endorsed/src/org.apache.sis.storage.netcdf/main/org/apache/sis/storage/netcdf/classic/ChannelDecoder.java @@ -270,7 +270,7 @@ public final class ChannelDecoder extends Decoder { if (tn != 0) { final int tag = (int) (tn >>> Integer.SIZE); final int nelems = (int) tn; - ensureNonNegative(nelems, tag); + ensureReasonableCount(nelems, tag); try { switch (tag) { case DIMENSION: dimensions = readDimensions(nelems); break; @@ -414,12 +414,25 @@ public final class ChannelDecoder extends Decoder { } /** - * Ensures that {@code nelems} is not a negative value. + * Ensures that {@code nelems} is not a negative value and not too large. + * The upper bound is a conservative estimation. It can detect only unreasonable values. + * We assume that all elements will require the space of at least one 32 bit integer. */ - private void ensureNonNegative(final int nelems, final int tag) throws DataStoreContentException { + private void ensureReasonableCount(final int nelems, final int tag) throws IOException, DataStoreContentException { + final short key; + final Object[] args; if (nelems < 0) { - throw new DataStoreContentException(errors().getString(Errors.Keys.NegativeArrayLength_1, tagPath(tagName(tag)))); + key = Errors.Keys.NegativeArrayLength_1; + args = new Object[1]; + } else if (Math.multiplyFull(nelems, Integer.SIZE) > input.remaining()) { + key = Errors.Keys.ExcessiveListSize_2; + args = new Object[2]; + args[1] = nelems; + } else { + return; } + args[0] = tagPath(tagName(tag)); + throw new DataStoreContentException(errors().getString(key, tagPath(tagName(tag)))); } /** @@ -568,7 +581,7 @@ public final class ChannelDecoder extends Decoder { * @return the dimensions in the order they are declared in the netCDF file. */ private DimensionInfo[] readDimensions(final int nelems) throws IOException, DataStoreContentException { - final DimensionInfo[] dimensions = new DimensionInfo[nelems]; + final var dimensions = new DimensionInfo[nelems]; for (int i=0; i<nelems; i++) { final String name = readName(); int length = input.readInt(); @@ -652,7 +665,7 @@ public final class ChannelDecoder extends Decoder { for (int j=0; j<nelems; j++) { final String name = readName(); final int n = input.readInt(); - final DimensionInfo[] varDims = new DimensionInfo[n]; + final var varDims = new DimensionInfo[n]; try { for (int i=0; i<n; i++) { varDims[i] = allDimensions[input.readInt()]; @@ -669,7 +682,7 @@ public final class ChannelDecoder extends Decoder { if (tn != 0) { final int tag = (int) (tn >>> Integer.SIZE); final int na = (int) tn; - ensureNonNegative(na, tag); + ensureReasonableCount(na, tag); switch (tag) { // More cases may be added later if they appear to exist. case ATTRIBUTE: { diff --git a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelData.java b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelData.java index e6ac52422c..56abbe677d 100644 --- a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelData.java +++ b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelData.java @@ -197,6 +197,13 @@ public abstract class ChannelData implements Markable { return false; } + /** + * Returns the number of bytes up to the last valid byte in the buffer. + */ + private long lengthOfBufferedData() { + return Math.addExact(bufferOffset, buffer.limit()); + } + /** * Returns the length of the stream (in bytes), or -1 if unknown. * The length is relative to the position during the last call to {@link #relocateOrigin()}. @@ -211,10 +218,10 @@ public abstract class ChannelData implements Markable { if (channel instanceof SeekableByteChannel) { final long length = Math.subtractExact(((SeekableByteChannel) channel).size(), channelOffset); if (length >= 0) { - return Math.max(length, Math.addExact(bufferOffset, buffer.limit())); + return Math.max(length, lengthOfBufferedData()); } } else if (isOpenedForAppend()) { - return Math.addExact(bufferOffset, buffer.limit()); + return lengthOfBufferedData(); } return -1; } diff --git a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelDataInput.java b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelDataInput.java index a07aacff72..64fc5c64d0 100644 --- a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelDataInput.java +++ b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelDataInput.java @@ -225,6 +225,24 @@ public class ChannelDataInput extends ChannelData implements DataInput { return c >= 0; } + /** + * Returns the remaining number of bytes, or {@code Long.MAX_VALUE} if unknown. + * This information is available only with instances of {@link SeekableByteChannel}. + * The {@link #hasRemaining()} method should be preferred when a count is not necessary. + * + * @return remaining number of bytes, or {@link Long#MAX_VALUE} if unknown. + * @throws IOException if an error occurred while fetching the channel length. + */ + public final long remaining() throws IOException { + if (channel instanceof SeekableByteChannel) { + final long length = ((SeekableByteChannel) channel).size(); + if (length >= 0) { + return length - Math.addExact(toSeekableByteChannelPosition(bufferOffset), buffer.position()); + } + } + return Long.MAX_VALUE; + } + /** * Makes sure that the buffer contains at least <var>n</var> remaining bytes. * It is caller's responsibility to ensure that the given number of bytes is
