Controls - [SOLVED]CXTPHexEdit crash when replacing Data buff

Print Page | Close Window

[SOLVED]CXTPHexEdit crash when replacing Data buff

Printed From: Codejock Forums
Category: Codejock Products
Forum Name: Controls
Forum Description: Topics Related to Codejock Controls
URL: http://forum.codejock.com/forum_posts.asp?TID=24202
Printed Date: 21 November 2024 at 9:24am
Software Version: Web Wiz Forums 12.04 - http://www.webwizforums.com

Topic: [SOLVED]CXTPHexEdit crash when replacing Data buff

Posted By: grumat
Subject: [SOLVED]CXTPHexEdit crash when replacing Data buff
Date Posted: 02 September 2021 at 4:19am

Hi,

I had a interesting scenario here:

An instantiated control displaying a 60K buffer.
Users scrolls up to the end of buffer and puts the cursor on the last position
User commands reload data
Crash inside CXTPHexEdit::OnDraw().

Reload Data does actually:

ctrl.SetData(NULL, 0);
Loads data
If data load OK: ctrl.SetData(newbuffer, newlength);

Problem is: The first command shrinks the buffer to 0.

The current implementation of CXTPHexEdit::SetData() does:

Replaces the buffer (m_pData) to the empty size
Calls SetSel(-1,-1)
Updates the indexes, including buffer bounds

Issue: SetSel causes repaint, and OnDraw ends up accessing invalid memory because buffer bounds refers to the old buffer size.

My current fix was to command:

ctrl..m_nLength = 0; // OMG! access to internal stuff!!

before the new SetData().

Surely that a definitive fix will needs more accuracy and tests, but hoping to see it on the next release.

Best regards,

M. Gruber

Replies:

Posted By: agontarenko
Date Posted: 03 September 2021 at 3:57am

Hello,

Unfortunately it is not clear what exactly is going wrong. Can you please provide more detailed information.
If it cannot be re-produced in any of our sample applications provided I would appreciate you sending a sample application so that we could debug it.

Regards,
Artem Gontarenko

Posted By: grumat
Date Posted: 03 September 2021 at 4:27am

Just see your source code:

XTPHexEdit.cpp:

void CXTPHexEdit::SetData(LPBYTE p, int nLength, int nMaxLength)
{
if (m_pData)

        free(m_pData);

    m_pData = (LPBYTE)malloc(nLength);
    MEMCPY_S(m_pData, p, nLength);

    SetSel(-1, -1);
    m_nMaxLength    = nMaxLength < 0 ? -1 : __max(nMaxLength, nLength);
    m_nLength         = nLength;
...

See the bold lines: You free the old buffer, then allocates the new buffer with the new length. Before updating the new buffer bound (m_nLength) you issue a SetSel, which "may" cause a WM_PAINT.

void CXTPHexEdit::SetSel(int nSelStart, int nSelEnd)
{

...

RedrawWindow();

...

}

The OnDraw checks m_nLength to keep access within the buffer bounds, but since the order of the previous code has a flaw, the m_nLength is not valid for the new buffer. This is critical if the new size of the buffer decreases. The use case requires an initial "big buffer" and also that we "scroll to the end of that buffer". So when a tiny new buffer is newly applied, it causes that all indexes for the current scroll position are outside of the new buffer bound, which will cause a CPU exception in the OnDraw:

void CXTPHexEdit::OnDraw(CDC* pDC)

...

if (m_pData)

...

if (m_bShowHex)
...

if (m_nSelStart != -1 && (m_eEditMode == editHigh || m_eEditMode == editLow))
...

else

...

for (i = m_nTopIndex; (i < m_nLength) && (rcd.TopLeft().y < height);)

...

TOHEX(m_pData[ i ], p);

This is the flow up to the access exception. Although m_nLength correctly guards the array bounds for the initial state, it actually doesn't during the SetData, because it refers to the "old control state".

Note: during my tests I had situations where a WM_PAINT did not happen (maybe Z-ordering), so the bug did not happen. Also CPU memory protection has a granularity with a minimum block size (4K, AFAIK), so the invalid access may not cause the exception in a 100% reproducible case. But the logic above has a clear flaw.

Hope this helps.

Posted By: agontarenko
Date Posted: 06 September 2021 at 3:35am

Hello,

I understood that you have problem. For a solution, I need to reproduce this bug.
Can you change HexEdit sample that the problem is reproducible, and attach it to topic.

Also, in v20.0 or 20.1 has been some changes, may be this problem already fixed.

Regards,
Artem Gontarenko

Posted By: grumat
Date Posted: 06 September 2021 at 3:44am

As you don't seem to understand: I have a fix for your bug and I am sharing it to you. There are enough information for a regular experienced programmer understand and fix the issue.

If you are not interested in the "continuous product improvement", then please close the issue. I won't annoy you anymore.

Posted By: agontarenko
Date Posted: 06 September 2021 at 4:35am

Ok, I changed our code. You right.

void CXTPHexEdit::SetData(LPBYTE p, int nLength, int nMaxLength)
{
   if (m_pData)
   {
        free(m_pData);

m_nLength = 0;

}

Thanks.