Skip to content

Commit 002fc51

Browse files
author
Deirdre Wang
committed
fix: Bound default buffer size and add decompression limits
1 parent a129203 commit 002fc51

11 files changed

Lines changed: 1819 additions & 24 deletions

‎src/main/java/com/amazon/ion/BufferConfiguration.java‎

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@
33

44
package com.amazon.ion;
55

6-
import com.amazon.ion.impl._Private_IonConstants;
7-
86
/**
97
* Provides logic common to all BufferConfiguration implementations.
108
* @param <Configuration> the type of the concrete subclass of this BufferConfiguration.
@@ -47,6 +45,12 @@ public static abstract class Builder<
4745
Configuration extends BufferConfiguration<Configuration>,
4846
BuilderType extends BufferConfiguration.Builder<Configuration, BuilderType>
4947
> {
48+
/**
49+
* The default maximum buffer size: 100 MB. This provides protection against memory exhaustion
50+
* from malicious inputs that declare excessively large lengths, while remaining large enough
51+
* for any realistic well-formed Ion stream.
52+
*/
53+
public static final int DEFAULT_MAXIMUM_BUFFER_SIZE = 100 * 1024 * 1024;
5054

5155
/**
5256
* Large enough that most streams will never need to grow the buffer. NOTE: this only needs to be large
@@ -62,7 +66,13 @@ public static abstract class Builder<
6266
/**
6367
* The maximum number of bytes that will be buffered.
6468
*/
65-
private int maximumBufferSize = _Private_IonConstants.ARRAY_MAXIMUM_SIZE;
69+
private int maximumBufferSize = DEFAULT_MAXIMUM_BUFFER_SIZE;
70+
71+
/**
72+
* Tracks whether the user has explicitly set the maximum buffer size via
73+
* {@link #withMaximumBufferSize(int)}.
74+
*/
75+
private boolean maximumBufferSizeExplicitlySet = false;
6676

6777
/**
6878
* The handler that will be notified when oversized values are encountered.
@@ -135,13 +145,14 @@ public final DataHandler getDataHandler() {
135145
* Set the maximum size of the buffer. For binary Ion, the minimum value is 5 because all valid binary Ion data
136146
* begins with a 4-byte Ion version marker and the smallest value is 1 byte. For text Ion, the minimum value is
137147
* 2 because the smallest text Ion value is 1 byte and the smallest delimiter is 1 byte.
138-
* Default: Near to the maximum size of an array.
148+
* Default: 100 MB ({@link #DEFAULT_MAXIMUM_BUFFER_SIZE}).
139149
*
140150
* @param maximumBufferSizeInBytes the value.
141151
* @return this builder.
142152
*/
143153
public final BuilderType withMaximumBufferSize(final int maximumBufferSizeInBytes) {
144154
maximumBufferSize = maximumBufferSizeInBytes;
155+
maximumBufferSizeExplicitlySet = true;
145156
return (BuilderType) this;
146157
}
147158

@@ -152,6 +163,14 @@ public int getMaximumBufferSize() {
152163
return maximumBufferSize;
153164
}
154165

166+
/**
167+
* @return true if the maximum buffer size was explicitly set by the user via
168+
* {@link #withMaximumBufferSize(int)}, false if using the default.
169+
*/
170+
public boolean isMaximumBufferSizeExplicitlySet() {
171+
return maximumBufferSizeExplicitlySet;
172+
}
173+
155174
/**
156175
* Gets the minimum allowed maximum buffer size.
157176
* @return the value.
@@ -216,7 +235,7 @@ protected BufferConfiguration(Builder<Configuration, ?> builder) {
216235
));
217236
}
218237
if (builder.getOversizedValueHandler() == null) {
219-
requireMaximumBufferSize();
238+
requireMaximumBufferSize(builder);
220239
oversizedValueHandler = builder.getThrowingOversizedValueHandler();
221240
} else {
222241
oversizedValueHandler = builder.getOversizedValueHandler();
@@ -229,10 +248,11 @@ protected BufferConfiguration(Builder<Configuration, ?> builder) {
229248
}
230249

231250
/**
232-
* Requires that the maximum buffer size not be limited.
251+
* Requires an OversizedValueHandler when the user explicitly sets a custom maximum buffer size.
252+
* When using the default, a throwing handler is applied automatically.
233253
*/
234-
private void requireMaximumBufferSize() {
235-
if (maximumBufferSize < _Private_IonConstants.ARRAY_MAXIMUM_SIZE) {
254+
private void requireMaximumBufferSize(Builder<Configuration, ?> builder) {
255+
if (builder.isMaximumBufferSizeExplicitlySet()) {
236256
throw new IllegalArgumentException(
237257
"Must specify an OversizedValueHandler when a custom maximum buffer size is specified."
238258
);

‎src/main/java/com/amazon/ion/IonBufferConfiguration.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@
33

44
package com.amazon.ion;
55

6-
import com.amazon.ion.impl._Private_IonConstants;
7-
86
/**
97
* Configures buffers that hold Ion data.
108
*/
@@ -180,18 +178,20 @@ public IonBufferConfiguration build() {
180178
private IonBufferConfiguration(Builder builder) {
181179
super(builder);
182180
if (builder.getOversizedSymbolTableHandler() == null) {
183-
requireMaximumBufferSize();
181+
requireMaximumBufferSize(builder);
184182
oversizedSymbolTableHandler = builder.getThrowingOversizedSymbolTableHandler();
185183
} else {
186184
oversizedSymbolTableHandler = builder.getOversizedSymbolTableHandler();
187185
}
188186
}
189187

190188
/**
191-
* Requires that the maximum buffer size not be limited.
189+
* Requires that the maximum buffer size not be limited unless the user explicitly configured it.
190+
* When the user explicitly sets a custom maximum buffer size, an OversizedSymbolTableHandler is required.
191+
* When using the default maximum buffer size, the throwing handler is used automatically.
192192
*/
193-
private void requireMaximumBufferSize() {
194-
if (getMaximumBufferSize() < _Private_IonConstants.ARRAY_MAXIMUM_SIZE) {
193+
private void requireMaximumBufferSize(Builder builder) {
194+
if (builder.isMaximumBufferSizeExplicitlySet()) {
195195
throw new IllegalArgumentException(
196196
"Must specify both an OversizedValueHandler and OversizedSymbolTableHandler when a custom maximum buffer size is specified."
197197
);

‎src/main/java/com/amazon/ion/impl/UnifiedInputStreamX.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// SPDX-License-Identifier: Apache-2.0
33
package com.amazon.ion.impl;
44

5+
import com.amazon.ion.IonException;
56
import com.amazon.ion.impl.UnifiedSavePointManagerX.SavePoint;
67
import java.io.Closeable;
78
import java.io.IOException;
@@ -77,6 +78,16 @@ abstract class UnifiedInputStreamX
7778
UnifiedSavePointManagerX _save_points;
7879

7980

81+
/**
82+
* The maximum total bytes that may be loaded across all pages, or -1 for no limit.
83+
*/
84+
long _maximumTotalBytes = -1;
85+
86+
/**
87+
* Tracks the total bytes loaded across all pages.
88+
*/
89+
long _totalBytesLoaded = 0;
90+
8091
// factories to construct an appropriate input stream
8192
// based on the input source
8293
public static UnifiedInputStreamX makeStream(CharSequence chars) {
@@ -94,6 +105,9 @@ public static UnifiedInputStreamX makeStream(char[] chars, int offset, int lengt
94105
public static UnifiedInputStreamX makeStream(Reader reader) throws IOException {
95106
return new FromCharStream(reader);
96107
}
108+
public static UnifiedInputStreamX makeStream(Reader reader, long maximumTotalBytes) throws IOException {
109+
return new FromCharStream(reader, maximumTotalBytes);
110+
}
97111
public static UnifiedInputStreamX makeStream(byte[] buffer) {
98112
return new FromByteArray(buffer, 0, buffer.length);
99113
}
@@ -103,6 +117,9 @@ public static UnifiedInputStreamX makeStream(byte[] buffer, int offset, int leng
103117
public static UnifiedInputStreamX makeStream(InputStream stream) throws IOException {
104118
return new FromByteStream(stream);
105119
}
120+
public static UnifiedInputStreamX makeStream(InputStream stream, long maximumTotalBytes) throws IOException {
121+
return new FromByteStream(stream, maximumTotalBytes);
122+
}
106123
public final InputStream getInputStream() { return _stream; }
107124
public final Reader getReader() { return _reader; }
108125
public final byte[] getByteArray() { return _bytes; }
@@ -498,6 +515,14 @@ protected final int load(UnifiedDataPageX curr, int start_pos, long file_positio
498515
else {
499516
read = curr.load(_reader, start_pos, file_position);
500517
}
518+
if (read > 0) {
519+
_totalBytesLoaded += read;
520+
if (_maximumTotalBytes > 0 && _totalBytesLoaded > _maximumTotalBytes) {
521+
throw new IonException(
522+
"Text stream exceeded maximum buffer size of " + _maximumTotalBytes + " bytes"
523+
);
524+
}
525+
}
501526
}
502527
return read;
503528
}
@@ -531,10 +556,16 @@ private static class FromCharArray extends UnifiedInputStreamX
531556
private static class FromCharStream extends UnifiedInputStreamX
532557
{
533558
FromCharStream(Reader reader) throws IOException
559+
{
560+
this(reader, -1);
561+
}
562+
563+
FromCharStream(Reader reader, long maximumTotalBytes) throws IOException
534564
{
535565
_is_byte_data = false;
536566
_is_stream = true;
537567
_reader = reader;
568+
_maximumTotalBytes = maximumTotalBytes;
538569
// If this page size ever becomes configurable watch out for _Private_IonConstants.ARRAY_MAXIMUM_SIZE
539570
_buffer = UnifiedInputBufferX.makePageBuffer(UnifiedInputBufferX.BufferType.CHARS, DEFAULT_PAGE_SIZE);
540571
super.init();
@@ -569,10 +600,16 @@ static class FromByteArray extends UnifiedInputStreamX
569600
private static class FromByteStream extends UnifiedInputStreamX
570601
{
571602
FromByteStream(InputStream stream) throws IOException
603+
{
604+
this(stream, -1);
605+
}
606+
607+
FromByteStream(InputStream stream, long maximumTotalBytes) throws IOException
572608
{
573609
_is_byte_data = true;
574610
_is_stream = true;
575611
_stream = stream;
612+
_maximumTotalBytes = maximumTotalBytes;
576613
// If this page size ever becomes configurable watch out for _Private_IonConstants.ARRAY_MAXIMUM_SIZE
577614
_buffer = UnifiedInputBufferX.makePageBuffer(UnifiedInputBufferX.BufferType.BYTES, DEFAULT_PAGE_SIZE);
578615
super.init();

‎src/main/java/com/amazon/ion/impl/_Private_IonReaderBuilder.java‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,14 @@
22
// SPDX-License-Identifier: Apache-2.0
33
package com.amazon.ion.impl;
44

5+
import com.amazon.ion.IonBufferConfiguration;
56
import com.amazon.ion.IonCatalog;
67
import com.amazon.ion.IonException;
78
import com.amazon.ion.IonReader;
89
import com.amazon.ion.IonTextReader;
910
import com.amazon.ion.IonValue;
1011
import com.amazon.ion.util.InputStreamInterceptor;
12+
import com.amazon.ion.util.LimitedInflaterInputStream;
1113
import com.amazon.ion.system.IonReaderBuilder;
1214
import com.amazon.ion.util.IonStreamUtils;
1315

@@ -205,6 +207,24 @@ private static void validateHeaderLength(int maxHeaderLength) {
205207
}
206208
}
207209

210+
/**
211+
* Wraps the given InputStream with a {@link LimitedInflaterInputStream} if the interceptor is for GZIP
212+
* and a finite maximum buffer size is configured.
213+
*/
214+
private static InputStream wrapWithInflationLimitIfNeeded(
215+
_Private_IonReaderBuilder builder,
216+
InputStreamInterceptor streamInterceptor,
217+
InputStream interceptedStream
218+
) {
219+
if ("gzip".equals(streamInterceptor.formatName())) {
220+
long maxDecompressedBytes = builder.getBufferConfiguration().getMaximumBufferSize();
221+
if (maxDecompressedBytes > 0) {
222+
return new LimitedInflaterInputStream(interceptedStream, maxDecompressedBytes);
223+
}
224+
}
225+
return interceptedStream;
226+
}
227+
208228
static IonReader buildReader(
209229
_Private_IonReaderBuilder builder,
210230
byte[] ionData,
@@ -222,9 +242,11 @@ static IonReader buildReader(
222242
}
223243
if (streamInterceptor.isMatch(ionData, offset, length)) {
224244
try {
245+
InputStream decompressedStream = streamInterceptor.newInputStream(new ByteArrayInputStream(ionData, offset, length));
246+
decompressedStream = wrapWithInflationLimitIfNeeded(builder, streamInterceptor, decompressedStream);
225247
return buildReader(
226248
builder,
227-
streamInterceptor.newInputStream(new ByteArrayInputStream(ionData, offset, length)),
249+
decompressedStream,
228250
_Private_IonReaderFactory::makeReaderBinary,
229251
_Private_IonReaderFactory::makeReaderText,
230252
// The builder provides only one level of detection, e.g. GZIP-compressed binary Ion *or*
@@ -275,7 +297,7 @@ private static boolean startsWithIvm(byte[] buffer, int length) {
275297

276298
@FunctionalInterface
277299
interface IonReaderFromInputStreamFactoryText {
278-
IonReader makeReader(IonCatalog catalog, InputStream source, _Private_LocalSymbolTableFactory lstFactory);
300+
IonReader makeReader(IonCatalog catalog, InputStream source, _Private_LocalSymbolTableFactory lstFactory, long maximumTotalBytes);
279301
}
280302

281303
@FunctionalInterface
@@ -357,6 +379,7 @@ static IonReader buildReader(
357379
ionData = streamInterceptor.newInputStream(
358380
new TwoElementInputStream(new ByteArrayInputStream(possibleIVM, 0, bytesRead), ionData)
359381
);
382+
ionData = wrapWithInflationLimitIfNeeded(builder, streamInterceptor, ionData);
360383
} catch (IOException e) {
361384
throw new IonException(e);
362385
}
@@ -376,7 +399,7 @@ static IonReader buildReader(
376399
} else {
377400
wrapper = ionData;
378401
}
379-
return text.makeReader(builder.validateCatalog(), wrapper, builder.lstFactory);
402+
return text.makeReader(builder.validateCatalog(), wrapper, builder.lstFactory, builder.getBufferConfiguration().getMaximumBufferSize());
380403
}
381404

382405
@Override
@@ -393,7 +416,7 @@ public IonReader build(InputStream source)
393416

394417
@Override
395418
public IonReader build(Reader ionText) {
396-
return makeReaderText(validateCatalog(), ionText, lstFactory);
419+
return makeReaderText(validateCatalog(), ionText, lstFactory, getBufferConfiguration().getMaximumBufferSize());
397420
}
398421

399422
@Override

‎src/main/java/com/amazon/ion/impl/_Private_IonReaderFactory.java‎

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,18 @@ public static final IonReader makeSystemReaderText(CharSequence chars)
8080
public static final IonReader makeReaderText(IonCatalog catalog,
8181
InputStream is,
8282
_Private_LocalSymbolTableFactory lstFactory)
83+
{
84+
return makeReaderText(catalog, is, lstFactory, -1);
85+
}
86+
87+
public static final IonReader makeReaderText(IonCatalog catalog,
88+
InputStream is,
89+
_Private_LocalSymbolTableFactory lstFactory,
90+
long maximumTotalBytes)
8391
{
8492
UnifiedInputStreamX uis;
8593
try {
86-
uis = makeUnifiedStream(is);
94+
uis = makeUnifiedStream(is, maximumTotalBytes);
8795
} catch (IOException e) {
8896
throw new IonException(e);
8997
}
@@ -104,12 +112,13 @@ public static IonReader makeSystemReaderText(InputStream is)
104112

105113
private static IonReader makeSystemReaderText(IonCatalog catalog,
106114
InputStream is,
107-
_Private_LocalSymbolTableFactory lstFactory)
115+
_Private_LocalSymbolTableFactory lstFactory,
116+
long maximumTotalBytes)
108117
{
109118
UnifiedInputStreamX uis;
110119
try
111120
{
112-
uis = makeUnifiedStream(is);
121+
uis = makeUnifiedStream(is, maximumTotalBytes);
113122
}
114123
catch (IOException e)
115124
{
@@ -138,9 +147,22 @@ private static IonReader makeSystemReaderText(IonCatalog catalog,
138147
public static final IonTextReader makeReaderText(IonCatalog catalog,
139148
Reader chars,
140149
_Private_LocalSymbolTableFactory lstFactory)
150+
{
151+
return makeReaderText(catalog, chars, lstFactory, -1);
152+
}
153+
154+
public static final IonTextReader makeReaderText(IonCatalog catalog,
155+
Reader chars,
156+
_Private_LocalSymbolTableFactory lstFactory,
157+
long maximumTotalBytes)
141158
{
142159
try {
143-
UnifiedInputStreamX in = makeStream(chars);
160+
UnifiedInputStreamX in;
161+
if (maximumTotalBytes > 0) {
162+
in = makeStream(chars, maximumTotalBytes);
163+
} else {
164+
in = makeStream(chars);
165+
}
144166
return new IonReaderTextUserX(catalog, lstFactory, in);
145167
}
146168
catch (IOException e) {
@@ -225,14 +247,19 @@ private static UnifiedInputStreamX makeUnifiedStream(byte[] bytes,
225247
return uis;
226248
}
227249

228-
private static UnifiedInputStreamX makeUnifiedStream(InputStream in)
250+
private static UnifiedInputStreamX makeUnifiedStream(InputStream in, long maximumTotalBytes)
229251
throws IOException
230252
{
231253
in.getClass(); // Force NPE
232254

233255
// TODO avoid multiple wrapping streams, use the UIS for the pushback
234256
in = IonStreamUtils.unGzip(in);
235-
UnifiedInputStreamX uis = UnifiedInputStreamX.makeStream(in);
257+
UnifiedInputStreamX uis;
258+
if (maximumTotalBytes > 0) {
259+
uis = UnifiedInputStreamX.makeStream(in, maximumTotalBytes);
260+
} else {
261+
uis = UnifiedInputStreamX.makeStream(in);
262+
}
236263
return uis;
237264
}
238265
}

0 commit comments

Comments
 (0)