From a9e879edf14d08f54adf5f712fb2a1c04e8bfc24 Mon Sep 17 00:00:00 2001 From: Richard Huang Date: Tue, 23 Jan 2018 17:08:15 +0000 Subject: [PATCH] Add defensive code to detect text memory buffer corruption Change-Id: I2a7114e95538822b8cacb9dfa3012df0f172d821 --- .../utc-Dali-Text-Controller.cpp | 35 +++++++++++++++++++++- .../internal/text/text-controller-impl.cpp | 30 +++++++++++++++++++ dali-toolkit/internal/text/text-controller-impl.h | 12 ++++++++ dali-toolkit/internal/text/text-controller.cpp | 16 ++++++++++ dali-toolkit/internal/text/text-controller.h | 5 +++- 5 files changed, 96 insertions(+), 2 deletions(-) diff --git a/automated-tests/src/dali-toolkit-internal/utc-Dali-Text-Controller.cpp b/automated-tests/src/dali-toolkit-internal/utc-Dali-Text-Controller.cpp index 6c63ce1..ad36607 100644 --- a/automated-tests/src/dali-toolkit-internal/utc-Dali-Text-Controller.cpp +++ b/automated-tests/src/dali-toolkit-internal/utc-Dali-Text-Controller.cpp @@ -26,6 +26,7 @@ #include #include #include +#include using namespace Dali; using namespace Toolkit; @@ -656,4 +657,36 @@ int UtcDaliTextControllerSetGetLineSpacingProperty(void) END_TEST; -} \ No newline at end of file +} + +int UtcDaliTextControllerCheckBufferIndices(void) +{ + tet_infoline(" UtcDaliTextControllerCheckBufferIndices"); + ToolkitTestApplication application; + + // Creates a text controller. + ControllerPtr controller = Controller::New(); + + ConfigureTextLabel(controller); + + // Set the text + const std::string text("A Quick Brown Fox Jumps Over The Lazy Dog"); + controller->SetText(text); + + // Get the implementation of the text controller + Controller::Impl& mImpl = Controller::Impl::GetImplementation( *controller.Get() ); + + // Tweak some parameters to make the indices to access the text buffer invalid + mImpl.mTextUpdateInfo.mNumberOfCharactersToAdd = mImpl.mModel->mLogicalModel->mText.Count() * 10u; + mImpl.mTextUpdateInfo.mNumberOfCharactersToRemove = 0u; + mImpl.mTextUpdateInfo.mPreviousNumberOfCharacters = 0u; + mImpl.mOperationsPending = Controller::ALL_OPERATIONS; + + // Perform a relayout + const Size size( Dali::Stage::GetCurrent().GetSize() ); + controller->Relayout(size); + + tet_result(TET_PASS); + + END_TEST; +} diff --git a/dali-toolkit/internal/text/text-controller-impl.cpp b/dali-toolkit/internal/text/text-controller-impl.cpp index 3e63578..b40dba5 100644 --- a/dali-toolkit/internal/text/text-controller-impl.cpp +++ b/dali-toolkit/internal/text/text-controller-impl.cpp @@ -811,6 +811,36 @@ bool Controller::Impl::UpdateModel( OperationsMask operationsRequired ) Length paragraphCharacters = 0u; CalculateTextUpdateIndices( paragraphCharacters ); + + // Check whether the indices for updating the text is valid + if ( numberOfCharacters > 0u && + ( mTextUpdateInfo.mParagraphCharacterIndex >= numberOfCharacters || + mTextUpdateInfo.mRequestedNumberOfCharacters > numberOfCharacters ) ) + { + std::string currentText; + Utf32ToUtf8( mModel->mLogicalModel->mText.Begin(), numberOfCharacters, currentText ); + + DALI_LOG_ERROR( "Controller::Impl::UpdateModel: mTextUpdateInfo has invalid indices\n" ); + DALI_LOG_ERROR( "Number of characters: %d, current text is: %s\n", numberOfCharacters, currentText.c_str() ); + + // Dump mTextUpdateInfo + DALI_LOG_ERROR( "Dump mTextUpdateInfo:\n" ); + DALI_LOG_ERROR( " mTextUpdateInfo.mCharacterIndex = %u\n", mTextUpdateInfo.mCharacterIndex ); + DALI_LOG_ERROR( " mTextUpdateInfo.mNumberOfCharactersToRemove = %u\n", mTextUpdateInfo.mNumberOfCharactersToRemove ); + DALI_LOG_ERROR( " mTextUpdateInfo.mNumberOfCharactersToAdd = %u\n", mTextUpdateInfo.mNumberOfCharactersToAdd ); + DALI_LOG_ERROR( " mTextUpdateInfo.mPreviousNumberOfCharacters = %u\n", mTextUpdateInfo.mPreviousNumberOfCharacters ); + DALI_LOG_ERROR( " mTextUpdateInfo.mParagraphCharacterIndex = %u\n", mTextUpdateInfo.mParagraphCharacterIndex ); + DALI_LOG_ERROR( " mTextUpdateInfo.mRequestedNumberOfCharacters = %u\n", mTextUpdateInfo.mRequestedNumberOfCharacters ); + DALI_LOG_ERROR( " mTextUpdateInfo.mStartGlyphIndex = %u\n", mTextUpdateInfo.mStartGlyphIndex ); + DALI_LOG_ERROR( " mTextUpdateInfo.mStartLineIndex = %u\n", mTextUpdateInfo.mStartLineIndex ); + DALI_LOG_ERROR( " mTextUpdateInfo.mEstimatedNumberOfLines = %u\n", mTextUpdateInfo.mEstimatedNumberOfLines ); + DALI_LOG_ERROR( " mTextUpdateInfo.mClearAll = %d\n", mTextUpdateInfo.mClearAll ); + DALI_LOG_ERROR( " mTextUpdateInfo.mFullRelayoutNeeded = %d\n", mTextUpdateInfo.mFullRelayoutNeeded ); + DALI_LOG_ERROR( " mTextUpdateInfo.mIsLastCharacterNewParagraph = %d\n", mTextUpdateInfo.mIsLastCharacterNewParagraph ); + + return false; + } + startIndex = mTextUpdateInfo.mParagraphCharacterIndex; if( mTextUpdateInfo.mClearAll || diff --git a/dali-toolkit/internal/text/text-controller-impl.h b/dali-toolkit/internal/text/text-controller-impl.h index f07e955..52dad25 100644 --- a/dali-toolkit/internal/text/text-controller-impl.h +++ b/dali-toolkit/internal/text/text-controller-impl.h @@ -712,6 +712,18 @@ struct Controller::Impl */ void ScrollTextToMatchCursor( const CursorInfo& cursorInfo ); +public: + + /** + * @brief Gets implementation from the controller handle. + * @param controller The text controller + * @return The implementation of the Controller + */ + static Impl& GetImplementation( Text::Controller& controller ) + { + return *controller.mImpl; + } + private: // Declared private and left undefined to avoid copies. Impl( const Impl& ); diff --git a/dali-toolkit/internal/text/text-controller.cpp b/dali-toolkit/internal/text/text-controller.cpp index 279f03e..136770c 100755 --- a/dali-toolkit/internal/text/text-controller.cpp +++ b/dali-toolkit/internal/text/text-controller.cpp @@ -3441,6 +3441,22 @@ bool Controller::DoRelayout( const Size& size, const CharacterIndex lastIndex = startIndex + ( ( requestedNumberOfCharacters > 0u ) ? requestedNumberOfCharacters - 1u : 0u ); const GlyphIndex startGlyphIndex = mImpl->mTextUpdateInfo.mStartGlyphIndex; + + // Make sure the index is not out of bound + if ( charactersToGlyph.Count() != glyphsPerCharacter.Count() || + requestedNumberOfCharacters > charactersToGlyph.Count() || + ( lastIndex >= charactersToGlyph.Count() && charactersToGlyph.Count() > 0u ) ) + { + std::string currentText; + GetText( currentText ); + + DALI_LOG_ERROR( "Controller::DoRelayout: Attempting to access invalid buffer\n" ); + DALI_LOG_ERROR( "Current text is: %s\n", currentText.c_str() ); + DALI_LOG_ERROR( "startIndex: %u, lastIndex: %u, requestedNumberOfCharacters: %u, charactersToGlyph.Count = %lu, glyphsPerCharacter.Count = %lu\n", startIndex, lastIndex, requestedNumberOfCharacters, charactersToGlyph.Count(), glyphsPerCharacter.Count()); + + return false; + } + const Length numberOfGlyphs = ( requestedNumberOfCharacters > 0u ) ? *( charactersToGlyphBuffer + lastIndex ) + *( glyphsPerCharacterBuffer + lastIndex ) - startGlyphIndex : 0u; const Length totalNumberOfGlyphs = mImpl->mModel->mVisualModel->mGlyphs.Count(); diff --git a/dali-toolkit/internal/text/text-controller.h b/dali-toolkit/internal/text/text-controller.h index 415b8f5..85c6818 100755 --- a/dali-toolkit/internal/text/text-controller.h +++ b/dali-toolkit/internal/text/text-controller.h @@ -1497,9 +1497,12 @@ protected: // Destructor. */ virtual ~Controller(); +public: + + struct Impl; ///< Made public for testing purposes + private: - struct Impl; Impl* mImpl; }; -- 2.7.4