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: 28 March 2024 at 10:08pm
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.



Print Page | Close Window

Forum Software by Web Wiz Forums® version 12.04 - http://www.webwizforums.com
Copyright ©2001-2021 Web Wiz Ltd. - https://www.webwiz.net