From f5a83adc01719b8409af12bd864e852bbb1de765 Mon Sep 17 00:00:00 2001 From: Martin Kiewitz Date: Tue, 9 Feb 2016 12:47:45 +0100 Subject: AGI: Fix various CIDs CID 1350104: regression from graphics rewrite in C64 picture drawing CID 1350101: potential buffer overflow in set.simple command CID 1350112: uninitialized variable in TextMgr CID 1350113: false positive uninitialized variable in SystemUI CID 1350114: potentially uninitialized variable in IIgsSample CID 1350117: false positive uninitialized variable in InventoryMgr CID 1350103: code bug in CGA rendering TextMgr::charAttrib_Set() CID 1350109: false positive in GfxFont::loadFontAmigaPseudoTopaz() CID 1350111: original AGI uninitialized memory issue in SpritesMgr::showObject --- engines/agi/font.cpp | 4 ++++ engines/agi/inv.cpp | 2 ++ engines/agi/op_cmd.cpp | 4 +++- engines/agi/picture.cpp | 6 +++--- engines/agi/sound_2gs.cpp | 2 ++ engines/agi/sprite.cpp | 2 +- engines/agi/systemui.cpp | 2 ++ engines/agi/text.cpp | 3 ++- 8 files changed, 19 insertions(+), 6 deletions(-) (limited to 'engines') diff --git a/engines/agi/font.cpp b/engines/agi/font.cpp index 670c1bf575..5c5ea51be8 100644 --- a/engines/agi/font.cpp +++ b/engines/agi/font.cpp @@ -829,6 +829,10 @@ void GfxFont::loadFontAmigaPseudoTopaz() { assert((topazBitOffset & 7) == 0); topazByteOffset = topazBitOffset >> 3; + + // Security check, although we are working on static const data from within ScummVM + assert((topazByteOffset + ((topazHeight - 1) * topazModulo)) < sizeof(fontData_AmigaPseudoTopaz)); + for (uint16 curHeight = 0; curHeight < topazHeight; curHeight++) { *fontData = topazData[topazByteOffset]; fontData++; diff --git a/engines/agi/inv.cpp b/engines/agi/inv.cpp index 1df5282622..834fa9badc 100644 --- a/engines/agi/inv.cpp +++ b/engines/agi/inv.cpp @@ -34,6 +34,8 @@ InventoryMgr::InventoryMgr(AgiEngine *agi, GfxMgr *gfx, TextMgr *text, SystemUI _gfx = gfx; _text = text; _systemUI = systemUI; + + _activeItemNr = -1; } InventoryMgr::~InventoryMgr() { diff --git a/engines/agi/op_cmd.cpp b/engines/agi/op_cmd.cpp index 2498c4c580..66386987a7 100644 --- a/engines/agi/op_cmd.cpp +++ b/engines/agi/op_cmd.cpp @@ -908,7 +908,9 @@ void cmdSetSimple(AgiGame *state, AgiEngine *vm, uint8 *parameter) { // Try to get description for automatic saves textPtr = state->strings[stringNr]; - strncpy(state->automaticSaveDescription, textPtr, sizeof(state->automaticSaveDescription)); + + memset(state->automaticSaveDescription, 0, sizeof(state->automaticSaveDescription)); + strncpy(state->automaticSaveDescription, textPtr, sizeof(state->automaticSaveDescription) - 1); if (state->automaticSaveDescription[0]) { // We got it and it's set, so enable automatic saving state->automaticSave = true; diff --git a/engines/agi/picture.cpp b/engines/agi/picture.cpp index 36eb587f68..0be2de7089 100644 --- a/engines/agi/picture.cpp +++ b/engines/agi/picture.cpp @@ -394,9 +394,6 @@ void PictureMgr::drawPictureC64() { _patCode = getNextByte(); plotBrush(); break; - case 0xfb: - draw_LineShort(); - break; case 0xff: // end of data return; default: @@ -433,6 +430,9 @@ void PictureMgr::drawPictureV1() { _scrOn = true; _priOn = false; break; + case 0xfb: + draw_LineShort(); + break; case 0xff: // end of data return; default: diff --git a/engines/agi/sound_2gs.cpp b/engines/agi/sound_2gs.cpp index b33591343a..b1bcee3920 100644 --- a/engines/agi/sound_2gs.cpp +++ b/engines/agi/sound_2gs.cpp @@ -482,6 +482,8 @@ static bool convertWave(Common::SeekableReadStream &source, int8 *dest, uint len IIgsSample::IIgsSample(uint8 *data, uint32 len, int16 resourceNr) : AgiSound() { Common::MemoryReadStream stream(data, len, DisposeAfterUse::YES); + _sample = nullptr; + // Check that the header was read ok and that it's of the correct type if (_header.read(stream) && _header.type == AGI_SOUND_SAMPLE) { // An Apple IIGS AGI sample resource uint32 sampleStartPos = stream.pos(); diff --git a/engines/agi/sprite.cpp b/engines/agi/sprite.cpp index 74f20c1d22..c68641fb33 100644 --- a/engines/agi/sprite.cpp +++ b/engines/agi/sprite.cpp @@ -394,7 +394,7 @@ void SpritesMgr::showObject(int16 viewNr) { screenObj.yPos_prev = SCRIPT_HEIGHT - 1; screenObj.yPos = screenObj.yPos_prev; screenObj.priority = 15; - screenObj.flags |= fFixedPriority; + screenObj.flags = fFixedPriority; // Original AGI did "| fFixedPriority" on uninitialized memory screenObj.objectNr = 255; // ??? backgroundBuffer = (uint8 *)malloc(screenObj.xSize * screenObj.ySize * 2); // for visual + priority data diff --git a/engines/agi/systemui.cpp b/engines/agi/systemui.cpp index 2dd6629103..42a35a8b33 100644 --- a/engines/agi/systemui.cpp +++ b/engines/agi/systemui.cpp @@ -38,6 +38,8 @@ SystemUI::SystemUI(AgiEngine *vm, GfxMgr *gfx, TextMgr *text) { _askForVerificationMouseLockedButtonNr = -1; _askForVerificationMouseActiveButtonNr = -1; + clearSavedGameSlots(); + _textStatusScore = "Score:%v3 of %v7"; _textStatusSoundOn = "Sound:on"; _textStatusSoundOff = "Sound:off"; diff --git a/engines/agi/text.cpp b/engines/agi/text.cpp index 62590737c3..7c8594b62a 100644 --- a/engines/agi/text.cpp +++ b/engines/agi/text.cpp @@ -66,6 +66,7 @@ TextMgr::TextMgr(AgiEngine *vm, Words *words, GfxMgr *gfx) { _inputStringRow = 0; _inputStringColumn = 0; + _inputStringEntered = false; _inputStringMaxLen = 0; _inputStringCursorPos = 0; _inputString[0] = 0; @@ -169,7 +170,7 @@ void TextMgr::charAttrib_Set(byte foreground, byte background) { if (background) { _textAttrib.combinedForeground = 3; _textAttrib.combinedBackground = 8; // enable invert of colors - } else if (foreground > 14) { + } else { if (foreground > 14) { _textAttrib.combinedForeground = 3; } else { -- cgit v1.2.3