From 239915200225fea4c5a02e9630044ef13fccb66d Mon Sep 17 00:00:00 2001 From: npm Date: Mon, 28 Nov 2016 12:49:29 -0800 Subject: Fix crash in CFDE_CSSSyntaxParser when parsing empty url When parsing "url('')", Subtract() should be called to correctly set m_iDatLen. But iLength will be 0 because there is no url. So I changed the ASSERT. Also replaced some non-const refs with pointers to make the code more readable. BUG=659509 Review-Url: https://codereview.chromium.org/2535663003 --- xfa/fde/css/fde_cssdatatable.cpp | 24 ++++++++++++------------ xfa/fde/css/fde_cssdatatable.h | 9 ++++----- xfa/fde/css/fde_cssdeclaration.cpp | 18 ++++++++++-------- xfa/fde/css/fde_csssyntax.cpp | 26 ++++++++++---------------- 4 files changed, 36 insertions(+), 41 deletions(-) diff --git a/xfa/fde/css/fde_cssdatatable.cpp b/xfa/fde/css/fde_cssdatatable.cpp index d2f81833e3..2f4841429d 100644 --- a/xfa/fde/css/fde_cssdatatable.cpp +++ b/xfa/fde/css/fde_cssdatatable.cpp @@ -698,31 +698,31 @@ bool FDE_ParseCSSNumber(const FX_WCHAR* pszValue, bool FDE_ParseCSSString(const FX_WCHAR* pszValue, int32_t iValueLen, - int32_t& iOffset, - int32_t& iLength) { + int32_t* iOffset, + int32_t* iLength) { ASSERT(pszValue && iValueLen > 0); - iOffset = 0; - iLength = iValueLen; + *iOffset = 0; + *iLength = iValueLen; if (iValueLen >= 2) { FX_WCHAR first = pszValue[0], last = pszValue[iValueLen - 1]; if ((first == '\"' && last == '\"') || (first == '\'' && last == '\'')) { - iOffset = 1, iLength -= 2; + *iOffset = 1; + *iLength -= 2; } } return iValueLen > 0; } bool FDE_ParseCSSURI(const FX_WCHAR* pszValue, - int32_t iValueLen, - int32_t& iOffset, - int32_t& iLength) { - ASSERT(pszValue && iValueLen > 0); - if (iValueLen < 6 || pszValue[iValueLen - 1] != ')' || + int32_t* iOffset, + int32_t* iLength) { + ASSERT(pszValue && *iLength > 0); + if (*iLength < 6 || pszValue[*iLength - 1] != ')' || FXSYS_wcsnicmp(L"url(", pszValue, 4)) { return false; } - if (FDE_ParseCSSString(pszValue + 4, iValueLen - 5, iOffset, iLength)) { - iOffset += 4; + if (FDE_ParseCSSString(pszValue + 4, *iLength - 5, iOffset, iLength)) { + *iOffset += 4; return true; } return false; diff --git a/xfa/fde/css/fde_cssdatatable.h b/xfa/fde/css/fde_cssdatatable.h index 660895cdb0..260432d9fc 100644 --- a/xfa/fde/css/fde_cssdatatable.h +++ b/xfa/fde/css/fde_cssdatatable.h @@ -161,14 +161,13 @@ bool FDE_ParseCSSNumber(const FX_WCHAR* pszValue, FDE_CSSPRIMITIVETYPE& eUnit); bool FDE_ParseCSSString(const FX_WCHAR* pszValue, int32_t iValueLen, - int32_t& iOffset, - int32_t& iLength); + int32_t* iOffset, + int32_t* iLength); bool FDE_ParseCSSColor(const FX_WCHAR* pszValue, int32_t iValueLen, FX_ARGB& dwColor); bool FDE_ParseCSSURI(const FX_WCHAR* pszValue, - int32_t iValueLen, - int32_t& iOffset, - int32_t& iLength); + int32_t* iOffset, + int32_t* iLength); #endif // XFA_FDE_CSS_FDE_CSSDATATABLE_H_ diff --git a/xfa/fde/css/fde_cssdeclaration.cpp b/xfa/fde/css/fde_cssdeclaration.cpp index 2196d6bd91..f6af900042 100644 --- a/xfa/fde/css/fde_cssdeclaration.cpp +++ b/xfa/fde/css/fde_cssdeclaration.cpp @@ -301,33 +301,35 @@ IFDE_CSSValue* CFDE_CSSDeclaration::ParseColor(const FDE_CSSPROPERTYARGS* pArgs, } return FXTARGET_NewWith(pArgs->pStaticStore) CFDE_CSSPrimitiveValue(dwColor); } + IFDE_CSSValue* CFDE_CSSDeclaration::ParseURI(const FDE_CSSPROPERTYARGS* pArgs, const FX_WCHAR* pszValue, int32_t iValueLen) { int32_t iOffset; - if (!FDE_ParseCSSURI(pszValue, iValueLen, iOffset, iValueLen)) { + if (!FDE_ParseCSSURI(pszValue, &iOffset, &iValueLen)) return nullptr; - } - if (iValueLen <= 0) { + + if (iValueLen <= 0) return nullptr; - } + pszValue = CopyToLocal(pArgs, pszValue + iOffset, iValueLen); return pszValue ? FXTARGET_NewWith(pArgs->pStaticStore) CFDE_CSSPrimitiveValue(FDE_CSSPRIMITIVETYPE_URI, pszValue) : nullptr; } + IFDE_CSSValue* CFDE_CSSDeclaration::ParseString( const FDE_CSSPROPERTYARGS* pArgs, const FX_WCHAR* pszValue, int32_t iValueLen) { int32_t iOffset; - if (!FDE_ParseCSSString(pszValue, iValueLen, iOffset, iValueLen)) { + if (!FDE_ParseCSSString(pszValue, iValueLen, &iOffset, &iValueLen)) return nullptr; - } - if (iValueLen <= 0) { + + if (iValueLen <= 0) return nullptr; - } + pszValue = CopyToLocal(pArgs, pszValue + iOffset, iValueLen); return pszValue ? FXTARGET_NewWith(pArgs->pStaticStore) diff --git a/xfa/fde/css/fde_csssyntax.cpp b/xfa/fde/css/fde_csssyntax.cpp index 436a94b67b..27094e1376 100644 --- a/xfa/fde/css/fde_csssyntax.cpp +++ b/xfa/fde/css/fde_csssyntax.cpp @@ -6,6 +6,8 @@ #include "xfa/fde/css/fde_csssyntax.h" +#include + #include "xfa/fde/css/fde_cssdatatable.h" #include "xfa/fgas/crt/fgas_codepage.h" @@ -280,16 +282,13 @@ FDE_CSSSYNTAXSTATUS CFDE_CSSSyntaxParser::DoSyntaxParse() { if (wch <= ' ' || wch == ';') { int32_t iURIStart, iURILength = m_TextData.GetLength(); - if (iURILength > 0 && - FDE_ParseCSSURI(m_TextData.GetBuffer(), iURILength, iURIStart, - iURILength)) { + if (iURILength > 0 && FDE_ParseCSSURI(m_TextData.GetBuffer(), + &iURIStart, &iURILength)) { m_TextData.Subtract(iURIStart, iURILength); SwitchMode(FDE_CSSSYNTAXMODE_MediaType); - if (IsImportEnabled()) { + if (IsImportEnabled()) return FDE_CSSSYNTAXSTATUS_URI; - } else { - break; - } + break; } } AppendChar(wch); @@ -468,15 +467,10 @@ bool CFDE_CSSTextBuf::ExpandBuf(int32_t iDesiredSize) { m_iBufLen = iDesiredSize; return true; } + void CFDE_CSSTextBuf::Subtract(int32_t iStart, int32_t iLength) { - ASSERT(iStart >= 0 && iLength > 0); - if (iLength > m_iDatLen - iStart) { - iLength = m_iDatLen - iStart; - } - if (iLength < 0) { - iLength = 0; - } else { - FXSYS_memmove(m_pBuffer, m_pBuffer + iStart, iLength * sizeof(FX_WCHAR)); - } + ASSERT(iStart >= 0 && iLength >= 0); + iLength = std::max(std::min(iLength, m_iDatLen - iStart), 0); + FXSYS_memmove(m_pBuffer, m_pBuffer + iStart, iLength * sizeof(FX_WCHAR)); m_iDatLen = iLength; } -- cgit v1.2.3