summaryrefslogtreecommitdiff
path: root/fpdfsdk
diff options
context:
space:
mode:
authorTom Sepez <tsepez@chromium.org>2014-07-30 13:03:52 -0700
committerTom Sepez <tsepez@chromium.org>2014-07-30 13:03:52 -0700
commit0d3b5cc6028550205b56a80ccdd81aecf67e4508 (patch)
tree9f9bd4975a11c4d817295b7d52e4dd39cc6eb6cf /fpdfsdk
parentcc965275f436267684a3f185ea3335e203cee6f9 (diff)
downloadpdfium-0d3b5cc6028550205b56a80ccdd81aecf67e4508.tar.xz
Speculative fix for uninitialized value in CFX_ByteString().
If somehow different length values could be obtained by two successive calls to Doc_getFilePath() (and FieldBrowse() for that matter), and the method is true to the API documentation that says "The return value always indicated number of bytes required for the buffer, even when there is no buffer specified, or the buffer size is less then required", then it is possible to get a returned length describing memory beyond the current buffer. We can make the corresponding JS_docGetFilePath() method more robust against this case by applying better checks to the returned value. This probably is unrelated since ASAN seems to be flagging the corresponding bug as UAF, but doesn't hurt to make things more robust. BUG=392956 R=jun_fang@foxitsoftware.com Review URL: https://codereview.chromium.org/423233002
Diffstat (limited to 'fpdfsdk')
-rw-r--r--fpdfsdk/include/fsdk_mgr.h50
1 files changed, 31 insertions, 19 deletions
diff --git a/fpdfsdk/include/fsdk_mgr.h b/fpdfsdk/include/fsdk_mgr.h
index d7e4e3d8a8..95d1a79352 100644
--- a/fpdfsdk/include/fsdk_mgr.h
+++ b/fpdfsdk/include/fsdk_mgr.h
@@ -173,18 +173,24 @@ public:
CFX_WideString JS_fieldBrowse()
{
- if(m_pInfo && m_pInfo->m_pJsPlatform && m_pInfo->m_pJsPlatform->Field_browse)
+ if (m_pInfo && m_pInfo->m_pJsPlatform && m_pInfo->m_pJsPlatform->Field_browse)
{
- int nLen = m_pInfo->m_pJsPlatform->Field_browse(m_pInfo->m_pJsPlatform, NULL, 0);
- if(nLen <= 0)
+ int nRequiredLen = m_pInfo->m_pJsPlatform->Field_browse(m_pInfo->m_pJsPlatform, NULL, 0);
+ if (nRequiredLen <= 0)
return L"";
- char* pbuff = new char[nLen];
- if(pbuff)
- memset(pbuff, 0, nLen);
- else
+
+ char* pbuff = new char[nRequiredLen];
+ if (!pbuff)
+ return L"";
+
+ memset(pbuff, 0, nRequiredLen);
+ int nActualLen = m_pInfo->m_pJsPlatform->Field_browse(m_pInfo->m_pJsPlatform, pbuff, nRequiredLen);
+ if (nActualLen <= 0 || nActualLen > nRequiredLen)
+ {
+ delete[] pbuff;
return L"";
- nLen = m_pInfo->m_pJsPlatform->Field_browse(m_pInfo->m_pJsPlatform, pbuff, nLen);
- CFX_ByteString bsRet = CFX_ByteString(pbuff, nLen);
+ }
+ CFX_ByteString bsRet = CFX_ByteString(pbuff, nActualLen);
CFX_WideString wsRet = CFX_WideString::FromLocal(bsRet);
delete[] pbuff;
return wsRet;
@@ -193,19 +199,25 @@ public:
}
CFX_WideString JS_docGetFilePath()
- {
- if(m_pInfo && m_pInfo->m_pJsPlatform && m_pInfo->m_pJsPlatform->Doc_getFilePath)
+ {
+ if (m_pInfo && m_pInfo->m_pJsPlatform && m_pInfo->m_pJsPlatform->Doc_getFilePath)
{
- int nLen = m_pInfo->m_pJsPlatform->Doc_getFilePath(m_pInfo->m_pJsPlatform, NULL, 0);
- if(nLen <= 0)
+ int nRequiredLen = m_pInfo->m_pJsPlatform->Doc_getFilePath(m_pInfo->m_pJsPlatform, NULL, 0);
+ if (nRequiredLen <= 0)
return L"";
- char* pbuff = new char[nLen];
- if(pbuff)
- memset(pbuff, 0, nLen);
- else
+
+ char* pbuff = new char[nRequiredLen];
+ if (!pbuff)
+ return L"";
+
+ memset(pbuff, 0, nRequiredLen);
+ int nActualLen = m_pInfo->m_pJsPlatform->Doc_getFilePath(m_pInfo->m_pJsPlatform, pbuff, nRequiredLen);
+ if (nActualLen <= 0 || nActualLen > nRequiredLen)
+ {
+ delete[] pbuff;
return L"";
- nLen = m_pInfo->m_pJsPlatform->Doc_getFilePath(m_pInfo->m_pJsPlatform, pbuff, nLen);
- CFX_ByteString bsRet = CFX_ByteString(pbuff, nLen);
+ }
+ CFX_ByteString bsRet = CFX_ByteString(pbuff, nActualLen);
CFX_WideString wsRet = CFX_WideString::FromLocal(bsRet);
delete[] pbuff;
return wsRet;