Mystic Star: Crash when attempting to open menu #1048

Closed
kakurasan opened this Issue Oct 2, 2016 · 3 comments

Comments

Projects
None yet
2 participants
@kakurasan
Contributor

kakurasan commented Oct 2, 2016

Name of the game:

Mystic Star

Player platform:

Ubuntu (Linux) x86_64

Attach files (as a .zip archive or link them)

Download: http://www.vector.co.jp/soft/dl/win95/game/se509140.html

  • unar is recommended to extract MysticStar.zip because some file names contain Japanese characters
  • This game doesn't require RTP

Describe the issue in detail and how to reproduce it:

When attempting to open menu, the program crashes with no useful output/log.

How to reproduce it:

  1. Start game
  2. Choose "NEW GAME" (Press DECISION key)
  3. Open menu (Press CANCEL key)

Menu screenshot (Wine + RPG_RT.exe):
wine-mysticstar-menu

@carstene1ns carstene1ns added the Crash label Oct 2, 2016

@carstene1ns

This comment has been minimized.

Show comment
Hide comment
@carstene1ns

carstene1ns Oct 2, 2016

Member

This is a buffer overflow in Bitmap::ToneBlit(). #1044 fixed some cases, but seems not all.

=6914==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61f000035480 at pc 0x000000524b8a bp 0x7ffc2d820800 sp 0x7ffc2d8207f0
READ of size 4 at 0x61f000035480 thread T0
    #0 0x524b89 in Bitmap::ToneBlit(int, int, Bitmap const&, Rect const&, Tone const&, Opacity const&) src/bitmap.cpp:1076
    #1 0x492794 in Sprite::Refresh(Rect&) src/sprite.cpp:163
    #2 0x4939c5 in Sprite::BlitScreen() src/sprite.cpp:84
    #3 0x434ee5 in Graphics::DrawFrame() src/graphics.cpp:185
    #4 0x43c05e in Player::Update(bool) src/player.cpp:292
    #5 0x46dc49 in Scene::MainFunction() src/scene.cpp:100
    #6 0x43cc5b in Player::MainLoop() src/player.cpp:257
    #7 0x446c94 in Player::Run() src/player.cpp:252
    #8 0x40a3d7 in main src/main.cpp:33
    #9 0x7fbb12c95290 in __libc_start_main (/usr/lib/libc.so.6+0x20290)
    #10 0x42c0f9 in _start (/home/carsten/projects/easyrpg/player/easyrpg-player+0x42c0f9)

0x61f000035480 is located 0 bytes to the right of 3072-byte region [0x61f000034880,0x61f000035480)
allocated by thread T0 here:
    #0 0x7fbb17f63020 in __interceptor_calloc /build/gcc-multilib/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:70
    #1 0x7fbb15745bf9  (/usr/lib/libpixman-1.so.0+0x19bf9)

SUMMARY: AddressSanitizer: heap-buffer-overflow src/bitmap.cpp:1076 in Bitmap::ToneBlit(int, int, Bitmap const&, Rect const&, Tone const&, Opacity const&)

Btw. funny coincidence: This issue with Mystic Star was also reported by mail recently.

Member

carstene1ns commented Oct 2, 2016

This is a buffer overflow in Bitmap::ToneBlit(). #1044 fixed some cases, but seems not all.

=6914==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61f000035480 at pc 0x000000524b8a bp 0x7ffc2d820800 sp 0x7ffc2d8207f0
READ of size 4 at 0x61f000035480 thread T0
    #0 0x524b89 in Bitmap::ToneBlit(int, int, Bitmap const&, Rect const&, Tone const&, Opacity const&) src/bitmap.cpp:1076
    #1 0x492794 in Sprite::Refresh(Rect&) src/sprite.cpp:163
    #2 0x4939c5 in Sprite::BlitScreen() src/sprite.cpp:84
    #3 0x434ee5 in Graphics::DrawFrame() src/graphics.cpp:185
    #4 0x43c05e in Player::Update(bool) src/player.cpp:292
    #5 0x46dc49 in Scene::MainFunction() src/scene.cpp:100
    #6 0x43cc5b in Player::MainLoop() src/player.cpp:257
    #7 0x446c94 in Player::Run() src/player.cpp:252
    #8 0x40a3d7 in main src/main.cpp:33
    #9 0x7fbb12c95290 in __libc_start_main (/usr/lib/libc.so.6+0x20290)
    #10 0x42c0f9 in _start (/home/carsten/projects/easyrpg/player/easyrpg-player+0x42c0f9)

0x61f000035480 is located 0 bytes to the right of 3072-byte region [0x61f000034880,0x61f000035480)
allocated by thread T0 here:
    #0 0x7fbb17f63020 in __interceptor_calloc /build/gcc-multilib/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:70
    #1 0x7fbb15745bf9  (/usr/lib/libpixman-1.so.0+0x19bf9)

SUMMARY: AddressSanitizer: heap-buffer-overflow src/bitmap.cpp:1076 in Bitmap::ToneBlit(int, int, Bitmap const&, Rect const&, Tone const&, Opacity const&)

Btw. funny coincidence: This issue with Mystic Star was also reported by mail recently.

@carstene1ns carstene1ns added the Bitmaps label Oct 2, 2016

@carstene1ns carstene1ns added this to the 0.5.1 milestone Oct 2, 2016

@carstene1ns

This comment has been minimized.

Show comment
Hide comment
@carstene1ns

carstene1ns Oct 2, 2016

Member

This seems to fix it. When we iterate over the pixels later in this function, we start at 0 and go until width-1 in the loop. However, the image has x==width==32, so we get beyond the array.

diff --git a/src/bitmap.cpp b/src/bitmap.cpp
index e1b63a7..5d7bd34 100644
--- a/src/bitmap.cpp
+++ b/src/bitmap.cpp
@@ -989,7 +989,7 @@ void Bitmap::ToneBlit(int x, int y, Bitmap const& src, Rect const& src_rect, con
        }

        // Only needed here, other codepaths are sanity checked by pixman
-       if (x < 0 || y < 0 || x > width() || y > height()) {
+       if (x < 0 || y < 0 || x >= width() || y >= height()) {
                return;
        }
Member

carstene1ns commented Oct 2, 2016

This seems to fix it. When we iterate over the pixels later in this function, we start at 0 and go until width-1 in the loop. However, the image has x==width==32, so we get beyond the array.

diff --git a/src/bitmap.cpp b/src/bitmap.cpp
index e1b63a7..5d7bd34 100644
--- a/src/bitmap.cpp
+++ b/src/bitmap.cpp
@@ -989,7 +989,7 @@ void Bitmap::ToneBlit(int x, int y, Bitmap const& src, Rect const& src_rect, con
        }

        // Only needed here, other codepaths are sanity checked by pixman
-       if (x < 0 || y < 0 || x > width() || y > height()) {
+       if (x < 0 || y < 0 || x >= width() || y >= height()) {
                return;
        }
@kakurasan

This comment has been minimized.

Show comment
Hide comment
@kakurasan

kakurasan Oct 3, 2016

Contributor

The patch works, thanks. I found another problem in this game and will open another issue.

Contributor

kakurasan commented Oct 3, 2016

The patch works, thanks. I found another problem in this game and will open another issue.

@Ghabry Ghabry closed this in a4dc6ad Oct 4, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment