From 3225b6fe66a84ed7f499daf84d085141a66bb346 Mon Sep 17 00:00:00 2001 From: Keisuke Kuroyanagi Date: Mon, 28 Jul 2014 21:48:00 +0900 Subject: [PATCH] Add boundary check for ver2 bigram reading. Bug: 16330528 Change-Id: I6aca6c7a735e2a652eb325572d44dff660789cff --- .../dictionary_bigrams_structure_policy.h | 2 +- .../v402/bigram/ver4_bigram_list_policy.h | 3 ++- .../bigram/bigram_list_read_write_utils.cpp | 21 +++++++++++++------ .../bigram/bigram_list_read_write_utils.h | 7 ++++--- .../structure/v2/bigram/bigram_list_policy.h | 17 ++++++++++----- .../structure/v2/patricia_trie_policy.cpp | 17 +++++++++++++-- .../structure/v2/patricia_trie_policy.h | 2 +- .../v4/bigram/ver4_bigram_list_policy.h | 3 ++- 8 files changed, 52 insertions(+), 20 deletions(-) diff --git a/native/jni/src/suggest/core/policy/dictionary_bigrams_structure_policy.h b/native/jni/src/suggest/core/policy/dictionary_bigrams_structure_policy.h index 661ef1b1a..aa0d068aa 100644 --- a/native/jni/src/suggest/core/policy/dictionary_bigrams_structure_policy.h +++ b/native/jni/src/suggest/core/policy/dictionary_bigrams_structure_policy.h @@ -30,7 +30,7 @@ class DictionaryBigramsStructurePolicy { virtual void getNextBigram(int *const outBigramPos, int *const outProbability, bool *const outHasNext, int *const pos) const = 0; - virtual void skipAllBigrams(int *const pos) const = 0; + virtual bool skipAllBigrams(int *const pos) const = 0; protected: DictionaryBigramsStructurePolicy() {} diff --git a/native/jni/src/suggest/policyimpl/dictionary/structure/backward/v402/bigram/ver4_bigram_list_policy.h b/native/jni/src/suggest/policyimpl/dictionary/structure/backward/v402/bigram/ver4_bigram_list_policy.h index 61623468e..50a4c9743 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/structure/backward/v402/bigram/ver4_bigram_list_policy.h +++ b/native/jni/src/suggest/policyimpl/dictionary/structure/backward/v402/bigram/ver4_bigram_list_policy.h @@ -58,8 +58,9 @@ class Ver4BigramListPolicy : public DictionaryBigramsStructurePolicy { void getNextBigram(int *const outBigramPos, int *const outProbability, bool *const outHasNext, int *const bigramEntryPos) const; - void skipAllBigrams(int *const pos) const { + bool skipAllBigrams(int *const pos) const { // Do nothing because we don't need to skip bigram lists in ver4 dictionaries. + return true; } bool addNewEntry(const int terminalId, const int newTargetTerminalId, diff --git a/native/jni/src/suggest/policyimpl/dictionary/structure/pt_common/bigram/bigram_list_read_write_utils.cpp b/native/jni/src/suggest/policyimpl/dictionary/structure/pt_common/bigram/bigram_list_read_write_utils.cpp index 08b4e0b5e..f7fd5c071 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/structure/pt_common/bigram/bigram_list_read_write_utils.cpp +++ b/native/jni/src/suggest/policyimpl/dictionary/structure/pt_common/bigram/bigram_list_read_write_utils.cpp @@ -38,9 +38,14 @@ const BigramListReadWriteUtils::BigramFlags BigramListReadWriteUtils::FLAG_ATTRI const BigramListReadWriteUtils::BigramFlags BigramListReadWriteUtils::MASK_ATTRIBUTE_PROBABILITY = 0x0F; -/* static */ void BigramListReadWriteUtils::getBigramEntryPropertiesAndAdvancePosition( - const uint8_t *const bigramsBuf, BigramFlags *const outBigramFlags, +/* static */ bool BigramListReadWriteUtils::getBigramEntryPropertiesAndAdvancePosition( + const uint8_t *const bigramsBuf, const int bufSize, BigramFlags *const outBigramFlags, int *const outTargetPtNodePos, int *const bigramEntryPos) { + if (bufSize <= *bigramEntryPos) { + AKLOGE("Read invalid pos in getBigramEntryPropertiesAndAdvancePosition(). bufSize: %d, " + "bigramEntryPos: %d.", bufSize, *bigramEntryPos); + return false; + } const BigramFlags bigramFlags = ByteArrayUtils::readUint8AndAdvancePosition(bigramsBuf, bigramEntryPos); if (outBigramFlags) { @@ -51,15 +56,19 @@ const BigramListReadWriteUtils::BigramFlags if (outTargetPtNodePos) { *outTargetPtNodePos = targetPos; } + return true; } -/* static */ void BigramListReadWriteUtils::skipExistingBigrams(const uint8_t *const bigramsBuf, - int *const bigramListPos) { +/* static */ bool BigramListReadWriteUtils::skipExistingBigrams(const uint8_t *const bigramsBuf, + const int bufSize, int *const bigramListPos) { BigramFlags flags; do { - getBigramEntryPropertiesAndAdvancePosition(bigramsBuf, &flags, 0 /* outTargetPtNodePos */, - bigramListPos); + if (!getBigramEntryPropertiesAndAdvancePosition(bigramsBuf, bufSize, &flags, + 0 /* outTargetPtNodePos */, bigramListPos)) { + return false; + } } while(hasNext(flags)); + return true; } /* static */ int BigramListReadWriteUtils::getBigramAddressAndAdvancePosition( diff --git a/native/jni/src/suggest/policyimpl/dictionary/structure/pt_common/bigram/bigram_list_read_write_utils.h b/native/jni/src/suggest/policyimpl/dictionary/structure/pt_common/bigram/bigram_list_read_write_utils.h index 15f924a6a..10f93fb7a 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/structure/pt_common/bigram/bigram_list_read_write_utils.h +++ b/native/jni/src/suggest/policyimpl/dictionary/structure/pt_common/bigram/bigram_list_read_write_utils.h @@ -30,8 +30,8 @@ class BigramListReadWriteUtils { public: typedef uint8_t BigramFlags; - static void getBigramEntryPropertiesAndAdvancePosition(const uint8_t *const bigramsBuf, - BigramFlags *const outBigramFlags, int *const outTargetPtNodePos, + static bool getBigramEntryPropertiesAndAdvancePosition(const uint8_t *const bigramsBuf, + const int bufSize, BigramFlags *const outBigramFlags, int *const outTargetPtNodePos, int *const bigramEntryPos); static AK_FORCE_INLINE int getProbabilityFromFlags(const BigramFlags flags) { @@ -43,7 +43,8 @@ public: } // Bigrams reading methods - static void skipExistingBigrams(const uint8_t *const bigramsBuf, int *const bigramListPos); + static bool skipExistingBigrams(const uint8_t *const bigramsBuf, const int bufSize, + int *const bigramListPos); private: DISALLOW_IMPLICIT_CONSTRUCTORS(BigramListReadWriteUtils); diff --git a/native/jni/src/suggest/policyimpl/dictionary/structure/v2/bigram/bigram_list_policy.h b/native/jni/src/suggest/policyimpl/dictionary/structure/v2/bigram/bigram_list_policy.h index 00bb502dc..73e291ec2 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/structure/v2/bigram/bigram_list_policy.h +++ b/native/jni/src/suggest/policyimpl/dictionary/structure/v2/bigram/bigram_list_policy.h @@ -27,27 +27,34 @@ namespace latinime { class BigramListPolicy : public DictionaryBigramsStructurePolicy { public: - explicit BigramListPolicy(const uint8_t *const bigramsBuf) : mBigramsBuf(bigramsBuf) {} + BigramListPolicy(const uint8_t *const bigramsBuf, const int bufSize) + : mBigramsBuf(bigramsBuf), mBufSize(bufSize) {} ~BigramListPolicy() {} void getNextBigram(int *const outBigramPos, int *const outProbability, bool *const outHasNext, int *const pos) const { BigramListReadWriteUtils::BigramFlags flags; - BigramListReadWriteUtils::getBigramEntryPropertiesAndAdvancePosition(mBigramsBuf, &flags, - outBigramPos, pos); + if (!BigramListReadWriteUtils::getBigramEntryPropertiesAndAdvancePosition(mBigramsBuf, + mBufSize, &flags, outBigramPos, pos)) { + AKLOGE("Cannot read bigram entry. mBufSize: %d, pos: %d. ", mBufSize, *pos); + *outProbability = NOT_A_PROBABILITY; + *outHasNext = false; + return; + } *outProbability = BigramListReadWriteUtils::getProbabilityFromFlags(flags); *outHasNext = BigramListReadWriteUtils::hasNext(flags); } - void skipAllBigrams(int *const pos) const { - BigramListReadWriteUtils::skipExistingBigrams(mBigramsBuf, pos); + bool skipAllBigrams(int *const pos) const { + return BigramListReadWriteUtils::skipExistingBigrams(mBigramsBuf, mBufSize, pos); } private: DISALLOW_IMPLICIT_CONSTRUCTORS(BigramListPolicy); const uint8_t *const mBigramsBuf; + const int mBufSize; }; } // namespace latinime #endif // LATINIME_BIGRAM_LIST_POLICY_H diff --git a/native/jni/src/suggest/policyimpl/dictionary/structure/v2/patricia_trie_policy.cpp b/native/jni/src/suggest/policyimpl/dictionary/structure/v2/patricia_trie_policy.cpp index 91d76040f..531cbb710 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/structure/v2/patricia_trie_policy.cpp +++ b/native/jni/src/suggest/policyimpl/dictionary/structure/v2/patricia_trie_policy.cpp @@ -223,7 +223,14 @@ int PatriciaTriePolicy::getCodePointsAndProbabilityAndReturnCodePointCount( mShortcutListPolicy.skipAllShortcuts(&pos); } if (PatriciaTrieReadingUtils::hasBigrams(flags)) { - mBigramListPolicy.skipAllBigrams(&pos); + if (!mBigramListPolicy.skipAllBigrams(&pos)) { + AKLOGE("Cannot skip bigrams. BufSize: %d, pos: %d.", mDictBufferSize, + pos); + mIsCorrupted = true; + ASSERT(false); + *outUnigramProbability = NOT_A_PROBABILITY; + return 0; + } } } } else { @@ -240,7 +247,13 @@ int PatriciaTriePolicy::getCodePointsAndProbabilityAndReturnCodePointCount( mShortcutListPolicy.skipAllShortcuts(&pos); } if (PatriciaTrieReadingUtils::hasBigrams(flags)) { - mBigramListPolicy.skipAllBigrams(&pos); + if (!mBigramListPolicy.skipAllBigrams(&pos)) { + AKLOGE("Cannot skip bigrams. BufSize: %d, pos: %d.", mDictBufferSize, pos); + mIsCorrupted = true; + ASSERT(false); + *outUnigramProbability = NOT_A_PROBABILITY; + return 0; + } } } diff --git a/native/jni/src/suggest/policyimpl/dictionary/structure/v2/patricia_trie_policy.h b/native/jni/src/suggest/policyimpl/dictionary/structure/v2/patricia_trie_policy.h index 7c0b9d3c5..d40b9af8d 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/structure/v2/patricia_trie_policy.h +++ b/native/jni/src/suggest/policyimpl/dictionary/structure/v2/patricia_trie_policy.h @@ -42,7 +42,7 @@ class PatriciaTriePolicy : public DictionaryStructureWithBufferPolicy { mHeaderPolicy(mMmappedBuffer->getBuffer(), FormatUtils::VERSION_2), mDictRoot(mMmappedBuffer->getBuffer() + mHeaderPolicy.getSize()), mDictBufferSize(mMmappedBuffer->getBufferSize() - mHeaderPolicy.getSize()), - mBigramListPolicy(mDictRoot), mShortcutListPolicy(mDictRoot), + mBigramListPolicy(mDictRoot, mDictBufferSize), mShortcutListPolicy(mDictRoot), mPtNodeReader(mDictRoot, mDictBufferSize, &mBigramListPolicy, &mShortcutListPolicy), mPtNodeArrayReader(mDictRoot, mDictBufferSize), mTerminalPtNodePositionsForIteratingWords(), mIsCorrupted(false) {} diff --git a/native/jni/src/suggest/policyimpl/dictionary/structure/v4/bigram/ver4_bigram_list_policy.h b/native/jni/src/suggest/policyimpl/dictionary/structure/v4/bigram/ver4_bigram_list_policy.h index 55ba613a5..4b3bb3725 100644 --- a/native/jni/src/suggest/policyimpl/dictionary/structure/v4/bigram/ver4_bigram_list_policy.h +++ b/native/jni/src/suggest/policyimpl/dictionary/structure/v4/bigram/ver4_bigram_list_policy.h @@ -40,8 +40,9 @@ class Ver4BigramListPolicy : public DictionaryBigramsStructurePolicy { void getNextBigram(int *const outBigramPos, int *const outProbability, bool *const outHasNext, int *const bigramEntryPos) const; - void skipAllBigrams(int *const pos) const { + bool skipAllBigrams(int *const pos) const { // Do nothing because we don't need to skip bigram lists in ver4 dictionaries. + return true; } bool addNewEntry(const int terminalId, const int newTargetTerminalId,