This is an automated email from the ASF dual-hosted git repository. rzo1 pushed a commit to branch OPENNLP-1899 in repository https://gitbox.apache.org/repos/asf/opennlp.git
commit 7210d1c15ab08f1fc5e6d2afd298227c9c11a2cc Author: Richard Zowalla <[email protected]> AuthorDate: Tue Jul 28 18:58:49 2026 +0200 OPENNLP-1899: Harden SymSpell model deserialization against oversized count fields --- .../tools/ml/model/AbstractModelReader.java | 5 +- .../dictionary/SymSpellModelSerializer.java | 11 ++- .../SymSpellModelSerializerLimitsTest.java | 109 +++++++++++++++++++++ 3 files changed, 120 insertions(+), 5 deletions(-) diff --git a/opennlp-core/opennlp-ml/opennlp-ml-commons/src/main/java/opennlp/tools/ml/model/AbstractModelReader.java b/opennlp-core/opennlp-ml/opennlp-ml-commons/src/main/java/opennlp/tools/ml/model/AbstractModelReader.java index d8ae3cb6f..8325b04f2 100644 --- a/opennlp-core/opennlp-ml/opennlp-ml-commons/src/main/java/opennlp/tools/ml/model/AbstractModelReader.java +++ b/opennlp-core/opennlp-ml/opennlp-ml-commons/src/main/java/opennlp/tools/ml/model/AbstractModelReader.java @@ -41,8 +41,11 @@ public abstract class AbstractModelReader { * Upper bound on count fields read from a model file. * Prevents OOM on crafted inputs with oversized array size declarations. * Configurable via the {@link #MAX_ENTRIES_PROPERTY} system property. + * <p> + * Public so that deserializers outside this package which implement their own binary + * format can apply the same bound to their count fields. */ - static final int MAX_ENTRIES = initMaxEntries(); + public static final int MAX_ENTRIES = initMaxEntries(); private static int initMaxEntries() { String prop = System.getProperty(MAX_ENTRIES_PROPERTY, "").trim(); diff --git a/opennlp-extensions/opennlp-spellcheck/src/main/java/opennlp/spellcheck/dictionary/SymSpellModelSerializer.java b/opennlp-extensions/opennlp-spellcheck/src/main/java/opennlp/spellcheck/dictionary/SymSpellModelSerializer.java index 39dee6d1d..27fbefb7b 100644 --- a/opennlp-extensions/opennlp-spellcheck/src/main/java/opennlp/spellcheck/dictionary/SymSpellModelSerializer.java +++ b/opennlp-extensions/opennlp-spellcheck/src/main/java/opennlp/spellcheck/dictionary/SymSpellModelSerializer.java @@ -31,6 +31,7 @@ import opennlp.spellcheck.distance.DamerauOSADistance; import opennlp.spellcheck.distance.EditDistance; import opennlp.spellcheck.distance.LevenshteinDistance; import opennlp.spellcheck.symspell.SymSpellConfig; +import opennlp.tools.ml.model.AbstractModelReader; import opennlp.tools.util.model.ArtifactSerializer; /** @@ -150,8 +151,9 @@ public final class SymSpellModelSerializer implements ArtifactSerializer<SymSpel } final int unigramCount = din.readInt(); - if (unigramCount < 0) { - throw new IOException("negative unigram count: " + unigramCount); + if (unigramCount < 0 || unigramCount > AbstractModelReader.MAX_ENTRIES) { + throw new IOException("unigram count " + unigramCount + + " exceeds safe limit of " + AbstractModelReader.MAX_ENTRIES); } final Map<String, Long> unigrams = LinkedHashMap.newLinkedHashMap(unigramCount); for (int i = 0; i < unigramCount; i++) { @@ -161,8 +163,9 @@ public final class SymSpellModelSerializer implements ArtifactSerializer<SymSpel } final int bigramCount = din.readInt(); - if (bigramCount < 0) { - throw new IOException("negative bigram count: " + bigramCount); + if (bigramCount < 0 || bigramCount > AbstractModelReader.MAX_ENTRIES) { + throw new IOException("bigram count " + bigramCount + + " exceeds safe limit of " + AbstractModelReader.MAX_ENTRIES); } final Map<String, Long> bigrams = LinkedHashMap.newLinkedHashMap(bigramCount); for (int i = 0; i < bigramCount; i++) { diff --git a/opennlp-extensions/opennlp-spellcheck/src/test/java/opennlp/spellcheck/dictionary/SymSpellModelSerializerLimitsTest.java b/opennlp-extensions/opennlp-spellcheck/src/test/java/opennlp/spellcheck/dictionary/SymSpellModelSerializerLimitsTest.java new file mode 100644 index 000000000..f809a673a --- /dev/null +++ b/opennlp-extensions/opennlp-spellcheck/src/test/java/opennlp/spellcheck/dictionary/SymSpellModelSerializerLimitsTest.java @@ -0,0 +1,109 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package opennlp.spellcheck.dictionary; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.DataOutputStream; +import java.io.IOException; + +import org.junit.jupiter.api.Test; + +import opennlp.tools.ml.model.AbstractModelReader; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Verifies that the unigram and bigram count fields of a SymSpell model stream are + * validated against {@link AbstractModelReader#MAX_ENTRIES} before the backing maps are + * pre-sized, so that a corrupt or out-of-range count fails loud instead of triggering an + * outsized allocation. + */ +class SymSpellModelSerializerLimitsTest { + + /** + * Writes a well-formed header up to (and including) the unigram count, optionally + * followed by a bigram count. A {@code null} bigram count stops after the unigram count. + */ + private static byte[] stream(int unigramCount, Integer bigramCount) throws IOException { + final ByteArrayOutputStream baos = new ByteArrayOutputStream(); + final DataOutputStream dout = new DataOutputStream(baos); + + dout.writeInt(SymSpellModelSerializer.MAGIC); + dout.writeInt(SymSpellModelSerializer.FORMAT_VERSION); + + dout.writeUTF("en"); + dout.writeUTF("test"); + dout.writeUTF("1.0"); + + dout.writeInt(2); // maxDictionaryEditDistance + dout.writeInt(7); // prefixLength + dout.writeLong(1L); // countThreshold + dout.writeUTF(SymSpellModelSerializer.EDIT_DISTANCE_DAMERAU_OSA); + dout.writeLong(0L); // corpusWordCount + + dout.writeInt(unigramCount); + if (bigramCount != null) { + dout.writeInt(bigramCount); + } + dout.flush(); + return baos.toByteArray(); + } + + private static IOException expectRejection(byte[] bytes) { + return assertThrows(IOException.class, + () -> new SymSpellModelSerializer().create(new ByteArrayInputStream(bytes))); + } + + @Test + void unigramCountMaxValueIsRejected() throws IOException { + final IOException e = expectRejection(stream(Integer.MAX_VALUE, null)); + assertTrue(e.getMessage().contains("unigram count"), e.getMessage()); + } + + @Test + void unigramCountAboveLimitIsRejected() throws IOException { + final IOException e = expectRejection(stream(AbstractModelReader.MAX_ENTRIES + 1, null)); + assertTrue(e.getMessage().contains("unigram count"), e.getMessage()); + } + + @Test + void negativeUnigramCountIsRejected() throws IOException { + final IOException e = expectRejection(stream(-1, null)); + assertTrue(e.getMessage().contains("unigram count"), e.getMessage()); + } + + @Test + void bigramCountMaxValueIsRejected() throws IOException { + final IOException e = expectRejection(stream(0, Integer.MAX_VALUE)); + assertTrue(e.getMessage().contains("bigram count"), e.getMessage()); + } + + @Test + void bigramCountAboveLimitIsRejected() throws IOException { + final IOException e = expectRejection(stream(0, AbstractModelReader.MAX_ENTRIES + 1)); + assertTrue(e.getMessage().contains("bigram count"), e.getMessage()); + } + + @Test + void negativeBigramCountIsRejected() throws IOException { + final IOException e = expectRejection(stream(0, -1)); + assertTrue(e.getMessage().contains("bigram count"), e.getMessage()); + } +}
