From 9919531f56a73e4a23a45958d1ce3f76d9d87324 Mon Sep 17 00:00:00 2001 From: Asdow <20314541+Asdow@users.noreply.github.com> Date: Mon, 7 Nov 2022 18:03:45 +0200 Subject: [PATCH 1/4] Fix INT16 buffer overflow when displaying items in items tab Original implementation initializes a ridiculously wide buffer where item graphics and names are then copied to and is then shown in parts depending on what items should be visible in item tab. When item counts go over ~32k in x-direction in pixel coordinates, the very low level drawing functions int16 variables overflow and item graphics are then drawn over each other. It's now changed to blit item graphics and names by on demand depending on the item indices, which does not require a huge buffer and works regardless of the amount of items, since only 11 are visible at any one time and the x-coordinate stays well within INT16 limits --- Editor/EditorItems.cpp | 169 +++++++++++++++++------------------------ 1 file changed, 71 insertions(+), 98 deletions(-) diff --git a/Editor/EditorItems.cpp b/Editor/EditorItems.cpp index a90fce7f..45716262 100644 --- a/Editor/EditorItems.cpp +++ b/Editor/EditorItems.cpp @@ -415,18 +415,16 @@ void InitEditorItemsInfo(UINT32 uiItemType) } else for( i = 0; i < eInfo.sNumItems; i++ ) { - + // Go through the items, and divide them into different classes. Store the item index for blitting later in RenderEditorItemsInfo() fTypeMatch = FALSE; while( usCounternotineditor) { usCounter++; @@ -495,92 +493,8 @@ void InitEditorItemsInfo(UINT32 uiItemType) } if( fTypeMatch ) { - - uiVideoObjectIndex = GetInterfaceGraphicForItem( item ); - GetVideoObject( &hVObject, uiVideoObjectIndex ); - //Store these item pointers for later when rendering selected items. eInfo.pusItemIndex[i] = usCounter; - - SetFont(SMALLCOMPFONT); - SetFontForeground( FONT_MCOLOR_WHITE ); - SetFontDestBuffer( eInfo.uiBuffer, 0, 0, eInfo.sWidth, eInfo.sHeight, FALSE ); - - - if( eInfo.uiItemType != TBAR_MODE_ITEM_TRIGGERS ) - { - LoadItemInfo( usCounter, pItemName, NULL ); - swprintf( pStr, L"%s", pItemName ); - } - else - { - if( i == PRESSURE_ACTION_ID ) - { - swprintf( pStr, pInitEditorItemsInfoText[0] ); - } - else if( i < 2 ) - { - if( usCounter == SWITCH ) - swprintf( pStr, pInitEditorItemsInfoText[5] ); - else - swprintf( pStr, pInitEditorItemsInfoText[1] ); - } - else if( i < 4 ) - { - if( usCounter == SWITCH ) - swprintf( pStr, pInitEditorItemsInfoText[6] ); - else - swprintf( pStr, pInitEditorItemsInfoText[2] ); - } - else if( i < 6 ) - { - if( usCounter == SWITCH ) - swprintf( pStr, pInitEditorItemsInfoText[7] ); - else - swprintf( pStr, pInitEditorItemsInfoText[3] ); - } - else - { - if( usCounter == SWITCH ) - swprintf( pStr, pInitEditorItemsInfoText[8], (i-4)/2 ); - else - swprintf( pStr, pInitEditorItemsInfoText[4], (i-4)/2 ); - } - } - - DisplayWrappedString(x, (UINT16)(y+25), 60, 2, SMALLCOMPFONT, FONT_WHITE, pStr, FONT_BLACK, TRUE, CENTER_JUSTIFIED ); - - UINT16 usGraphicNum = g_bUsePngItemImages ? 0 : item->ubGraphicNum; - if(usGraphicNum < hVObject->usNumberOfObjects) - { - //Calculate the center position of the graphic in a 60 pixel wide area. - sWidth = hVObject->pETRLEObject[usGraphicNum].usWidth; - sOffset = hVObject->pETRLEObject[usGraphicNum].sOffsetX; - sStart = x + (60 - sWidth - sOffset*2) / 2; - - if( sWidth && sWidth > 0 ) - { - BltVideoObjectOutlineFromIndex( eInfo.uiBuffer, uiVideoObjectIndex, usGraphicNum, sStart, y+2, 0, FALSE ); - } - - //cycle through the various slot positions (0,0), (0,40), (60,0), (60,40), (120,0)... - if( y == 0 ) - { - y = 40; - } - else - { - y = 0; - x += 60; - } - } - else - { - static vfs::Log& editorLog = *vfs::Log::create(L"EditorItems.log"); - editorLog << L"Tried to access item [" - << item->ubGraphicNum << L"/" << hVObject->usNumberOfObjects - << L"]" << vfs::Log::endl; - } } usCounter++; } @@ -627,14 +541,6 @@ void RenderEditorItemsInfo() { //Mouse has moved out of the items display region -- so nothing can be highlighted. eInfo.sHilitedItemIndex = -1; } - pDestBuf = LockVideoSurface(FRAME_BUFFER, &uiDestPitchBYTES); - pSrcBuf = LockVideoSurface(eInfo.uiBuffer, &uiSrcPitchBYTES); - - Blt16BPPTo16BPP((UINT16 *)pDestBuf, uiDestPitchBYTES, - (UINT16 *)pSrcBuf, uiSrcPitchBYTES, iScreenWidthOffset + 110, 2 * iScreenHeightOffset + 360, 60*eInfo.sScrollIndex, 0, 360, 80 ); - - UnLockVideoSurface(eInfo.uiBuffer); - UnLockVideoSurface(FRAME_BUFFER); //calculate the min and max index that is currently shown. This determines //if the highlighted and/or selected items are drawn with the outlines. @@ -691,10 +597,77 @@ void RenderEditorItemsInfo() { x = iScreenWidthOffset + (i/2 - eInfo.sScrollIndex)*60 + 110; y = 2 * iScreenHeightOffset + 360 + (i % 2) * 40; - SetFont( BLOCKFONTNARROW ); - SetFontForeground( FONT_LTGREEN ); - SetFontShadow( FONT_NEARBLACK ); + // Blit item graphics on demand based on what is visible + UINT16 itemIndex = eInfo.pusItemIndex[i]; + item = &Item[itemIndex]; + uiVideoObjectIndex = GetInterfaceGraphicForItem(item); + GetVideoObject(&hVObject, uiVideoObjectIndex); + UINT16 usGraphicNum = g_bUsePngItemImages ? 0 : item->ubGraphicNum; + if (usGraphicNum >= hVObject->usNumberOfObjects) // Tried to access graphics outside of hvObject's indices + { + static vfs::Log& editorLog = *vfs::Log::create(L"EditorItems.log"); + editorLog << L"Tried to access item [" + << item->ubGraphicNum << L"/" << hVObject->usNumberOfObjects + << L"]" << vfs::Log::endl; + } + sWidth = hVObject->pETRLEObject[usGraphicNum].usWidth; + sOffset = hVObject->pETRLEObject[usGraphicNum].sOffsetX; + sStart = x + (60 - sWidth - sOffset * 2) / 2; + BltVideoObjectOutlineFromIndex(FRAME_BUFFER, uiVideoObjectIndex, usGraphicNum, sStart, y + 2, Get16BPPColor(FROMRGB(250, 0, 0)), FALSE); + // Display item name + CHAR16 pStr[100]; + CHAR16 pItemName[SIZE_ITEM_NAME]; + + if (eInfo.uiItemType != TBAR_MODE_ITEM_TRIGGERS) + { + LoadItemInfo(eInfo.pusItemIndex[i], pItemName, NULL); + swprintf(pStr, L"%s", pItemName); + } + else + { + if (i == PRESSURE_ACTION_ID) + { + swprintf(pStr, pInitEditorItemsInfoText[0]); + } + else if (i < 2) + { + if (itemIndex == SWITCH) + swprintf(pStr, pInitEditorItemsInfoText[5]); + else + swprintf(pStr, pInitEditorItemsInfoText[1]); + } + else if (i < 4) + { + if (itemIndex == SWITCH) + swprintf(pStr, pInitEditorItemsInfoText[6]); + else + swprintf(pStr, pInitEditorItemsInfoText[2]); + } + else if (i < 6) + { + if (itemIndex == SWITCH) + swprintf(pStr, pInitEditorItemsInfoText[7]); + else + swprintf(pStr, pInitEditorItemsInfoText[3]); + } + else + { + if (itemIndex == SWITCH) + swprintf(pStr, pInitEditorItemsInfoText[8], (i - 4) / 2); + else + swprintf(pStr, pInitEditorItemsInfoText[4], (i - 4) / 2); + } + } + SetFont(SMALLCOMPFONT); + SetFontForeground(FONT_MCOLOR_WHITE); + DisplayWrappedString(x, (UINT16)(y + 25), 60, 2, SMALLCOMPFONT, FONT_WHITE, pStr, FONT_BLACK, TRUE, CENTER_JUSTIFIED); + + + + SetFont(BLOCKFONTNARROW); + SetFontForeground(FONT_LTGREEN); + SetFontShadow(FONT_NEARBLACK); // item index no mprintf( x + 12, y + 18, L"%d", eInfo.pusItemIndex[ i ] ); From af9e2215782717d6c5aded0d90fc93fdb248f3ec Mon Sep 17 00:00:00 2001 From: Asdow <20314541+Asdow@users.noreply.github.com> Date: Mon, 7 Nov 2022 19:12:53 +0200 Subject: [PATCH 2/4] Removed remnants of editoritems buffer as it is no longer used --- Editor/EditorItems.cpp | 113 ++++------------------------------------- Editor/EditorItems.h | 5 -- 2 files changed, 10 insertions(+), 108 deletions(-) diff --git a/Editor/EditorItems.cpp b/Editor/EditorItems.cpp index 45716262..60afbaf2 100644 --- a/Editor/EditorItems.cpp +++ b/Editor/EditorItems.cpp @@ -137,11 +137,7 @@ void EntryInitEditorItemsInfo() { INT32 i; INVTYPE *item; - eInfo.uiBuffer = 0; - eInfo.fKill = 0; eInfo.fActive = 0; - eInfo.sWidth = 0; - eInfo.sHeight = 0; eInfo.sScrollIndex = 0; eInfo.sSelItemIndex = 0; eInfo.sHilitedItemIndex = -1; @@ -223,20 +219,10 @@ void EntryInitEditorItemsInfo() void InitEditorItemsInfo(UINT32 uiItemType) { - VSURFACE_DESC vs_desc; - UINT8 *pDestBuf, *pSrcBuf; - UINT32 uiSrcPitchBYTES, uiDestPitchBYTES; INVTYPE *item; - SGPRect SaveRect, NewRect; - HVOBJECT hVObject; - UINT32 uiVideoObjectIndex; - UINT16 usUselessWidth, usUselessHeight; - INT32 sWidth, sOffset, sStart; - INT32 i, x, y; + SGPRect SaveRect; + INT32 i; UINT16 usCounter; - CHAR16 pStr[ 100 ];//, pStr2[ 100 ]; - CHAR16 pItemName[SIZE_ITEM_NAME]; - UINT8 ubBitDepth; BOOLEAN fTypeMatch; INT32 iEquipCount = 0; @@ -327,95 +313,23 @@ void InitEditorItemsInfo(UINT32 uiItemType) //Allocate memory to store all the item pointers. if(eInfo.sNumItems)//dnl ch78 271113 eInfo.pusItemIndex = (UINT16*)MemAlloc( sizeof(UINT16) * eInfo.sNumItems ); + //Disable the appropriate scroll buttons based on the saved scroll index if applicable - //Left most scroll position DetermineItemsScrolling(); - //calculate the width of the buffer based on the number of items. - //every pair of items (odd rounded up) requires 60 pixels for width. - //the minimum buffer size is 420. Height is always 80 pixels. - eInfo.sWidth = (eInfo.sNumItems > 12) ? (((INT32) eInfo.sNumItems+1)/2)*60 : 360;//dnl ch77 251113 was (SCREEN_HEIGHT - 120) but editor crash if resolution is 1024x768 - eInfo.sHeight = 80; - // Create item buffer - GetCurrentVideoSettings( &usUselessWidth, &usUselessHeight, &ubBitDepth ); - vs_desc.fCreateFlags = VSURFACE_CREATE_DEFAULT | VSURFACE_SYSTEM_MEM_USAGE; - vs_desc.usWidth = eInfo.sWidth; - vs_desc.usHeight = eInfo.sHeight; - vs_desc.ubBitDepth = ubBitDepth; - - //!!!Memory check. Create the item buffer - if(!AddVideoSurface( &vs_desc, &eInfo.uiBuffer )) - { - eInfo.fKill = TRUE; - eInfo.fActive = FALSE; - return; - } - - pDestBuf = LockVideoSurface(eInfo.uiBuffer, &uiDestPitchBYTES); - pSrcBuf = LockVideoSurface(FRAME_BUFFER, &uiSrcPitchBYTES); - - //copy a blank chunk of the editor interface to the new buffer. - for( i=0; iubGraphicNum; - //Calculate the center position of the graphic in a 60 pixel wide area. - sWidth = hVObject->pETRLEObject[usGraphicNum].usWidth; - sOffset = hVObject->pETRLEObject[usGraphicNum].sOffsetX; - sStart = x + (60 - sWidth - sOffset*2) / 2; - - BltVideoObjectOutlineFromIndex( eInfo.uiBuffer, uiVideoObjectIndex, usGraphicNum, sStart, y+2, 0, FALSE ); - - //cycle through the various slot positions (0,0), (0,40), (60,0), (60,40), (120,0)... - if( y == 0 ) - { - y = 40; - } - else - { - y = 0; - x += 60; - } + eInfo.pusItemIndex[i] = KeyTable[LockTable[i].usKeyItem].usItem; } } else for( i = 0; i < eInfo.sNumItems; i++ ) { - // Go through the items, and divide them into different classes. Store the item index for blitting later in RenderEditorItemsInfo() fTypeMatch = FALSE; while( usCounter Date: Mon, 7 Nov 2022 19:42:08 +0200 Subject: [PATCH 3/4] Update EditorItems.cpp Improve readability --- Editor/EditorItems.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/Editor/EditorItems.cpp b/Editor/EditorItems.cpp index 60afbaf2..8301eed3 100644 --- a/Editor/EditorItems.cpp +++ b/Editor/EditorItems.cpp @@ -502,13 +502,16 @@ void RenderEditorItemsInfo() } } } - //draw item index no & the numbers of each visible item that currently resides in the world. + + + //draw item graphics, name, index number & the numbers of each visible item that currently resides in the world. maxIndex = min( maxIndex, eInfo.sNumItems-1 ); for( i = minIndex; i <= maxIndex; i++ ) { x = iScreenWidthOffset + (i/2 - eInfo.sScrollIndex)*60 + 110; y = 2 * iScreenHeightOffset + 360 + (i % 2) * 40; + // Blit item graphics on demand based on what is visible UINT16 itemIndex = eInfo.pusItemIndex[i]; item = &Item[itemIndex]; @@ -526,6 +529,8 @@ void RenderEditorItemsInfo() sOffset = hVObject->pETRLEObject[usGraphicNum].sOffsetX; sStart = x + (60 - sWidth - sOffset * 2) / 2; BltVideoObjectOutlineFromIndex(FRAME_BUFFER, uiVideoObjectIndex, usGraphicNum, sStart, y + 2, Get16BPPColor(FROMRGB(250, 0, 0)), FALSE); + + // Display item name CHAR16 pStr[100]; CHAR16 pItemName[SIZE_ITEM_NAME]; @@ -583,16 +588,15 @@ void RenderEditorItemsInfo() SetFont(BLOCKFONTNARROW); SetFontForeground(FONT_LTGREEN); SetFontShadow(FONT_NEARBLACK); - // item index no + // item index number mprintf( x + 12, y + 18, L"%d", eInfo.pusItemIndex[ i ] ); - // index no within usItemClass + // index number within usItemClass SetFontForeground( FONT_LTBLUE ); mprintf( x + 40, y + 18, L"%d", i ); // numbers of each visible item usNumItems = CountNumberOfEditorPlacementsInWorld( i, &usQuantity ); - if( usNumItems ) { SetFont( FONT10ARIAL ); From 06065bb62f2bfd2ba4a0bdd6e6cabb13330e2c21 Mon Sep 17 00:00:00 2001 From: Asdow <20314541+Asdow@users.noreply.github.com> Date: Mon, 7 Nov 2022 19:53:40 +0200 Subject: [PATCH 4/4] Update EditorItems.cpp Removed unused clippingRect --- Editor/EditorItems.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Editor/EditorItems.cpp b/Editor/EditorItems.cpp index 8301eed3..db7d61cf 100644 --- a/Editor/EditorItems.cpp +++ b/Editor/EditorItems.cpp @@ -220,7 +220,6 @@ void EntryInitEditorItemsInfo() void InitEditorItemsInfo(UINT32 uiItemType) { INVTYPE *item; - SGPRect SaveRect; INT32 i; UINT16 usCounter; BOOLEAN fTypeMatch; @@ -318,7 +317,6 @@ void InitEditorItemsInfo(UINT32 uiItemType) DetermineItemsScrolling(); usCounter = 0; - GetClippingRect(&SaveRect); // Go through the items, and depending on the current eInfo.uiItemType, store the item indexes for blitting graphics and names later in RenderEditorItemsInfo() if( eInfo.uiItemType == TBAR_MODE_ITEM_KEYS ) { //Keys use a totally different method for determining @@ -413,8 +411,6 @@ void InitEditorItemsInfo(UINT32 uiItemType) usCounter++; } } - SetClippingRect(&SaveRect); - gfRenderTaskbar = TRUE; } void DetermineItemsScrolling()