Bug#440124: Buffer overflow in ONScripterLabel_rmenu.cpp
Dear Bryan, Can you provide the information about what game makes the program crash? Probably with your save file if you don't mind :) Or how can we make a script from scratch to crash the program? I'd like to reproduce this bug by myself because I'm not very sure this is a bug. Yes, as you mentioned, each numeric characters (double-width ０１２３４５６７８９) in Shift-JIS is two bytes long. But when compiling with ENABLE_1BYTE_CHAR and FORCE_1BYTE_CHAR, the data stored in save_file_info.* are pure ASCII numeric characters (single-width 0123456789). Thus, even each field of save_file_info.* is 5 bytes long, only 3 of them is used when using ENABLE_1BYTE_CHAR and FORCE_1BYTE_CHAR. (two digits and NUL terminator) Please take a look at ScriptHandler::getStringFromInteger() in ScriptHandler.cpp which is used to write to the fields of save_file_info. Thanks, Ying-Chun Liu -- PaulLiu(劉穎駿) E-mail address: [EMAIL PROTECTED] signature.asc Description: OpenPGP digital signature
Bug#440124: Buffer overflow in ONScripterLabel_rmenu.cpp
On 8/30/07, Ying-Chun Liu (PaulLiu) [EMAIL PROTECTED] wrote: Dear Bryan, Can you provide the information about what game makes the program crash? Probably with your save file if you don't mind :) The game is the english patch of higurashi kai ( http://hinamizawaclub.com ). The crash was intermittent - maybe half of the time I opened a save or load dialog with some save files extant a crash would occur. I don't have a save at the moment (I've been patching my copy of onscripter locally) but I'll see if I can get a crash and core dump with the version in sid. Or how can we make a script from scratch to crash the program? I'd like to reproduce this bug by myself because I'm not very sure this is a bug. Yes, as you mentioned, each numeric characters (double-width ０１２３４５６７８９) in Shift-JIS is two bytes long. But when compiling with ENABLE_1BYTE_CHAR and FORCE_1BYTE_CHAR, the data stored in save_file_info.* are pure ASCII numeric characters (single-width 0123456789). Thus, even each field of save_file_info.* is 5 bytes long, only 3 of them is used when using ENABLE_1BYTE_CHAR and FORCE_1BYTE_CHAR. (two digits and NUL terminator) That should be enough: 21 bytes (MESSAGE_SAVE_EXIST minus format strings and NUL) + 2 * 5 + 1 (the NUL) = 32, which is larger than the allocated buffer. Moreover, if a malicious savefile uses more than two bytes in each field, you'll have a possibly-exploitable buffer overflow for sure. In any case, using hardcoded constants seems dangerous, especially when they're calculated so close to the absolute minimum. RAM's cheap, throw a few hundred bytes at it to make the problem go away for sure :) -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#440124: Buffer overflow in ONScripterLabel_rmenu.cpp
Package: onscripter
Version: 0.0.20070826a-1
Severity: normal
In ONScripterLabel_rmenu.cpp, in ONScripterLabel::executeSystemLoad(),
the following code has a buffer overflow when defined(ENABLE_1BYTE_CHAR)
defined(FORCE_1BYTE_CHAR):
char *buffer = new char[ strlen( save_item_name ) + 30 + 1 ];
for ( unsigned int i=1 ; i=num_save_file ; i++ ){
searchSaveFile( save_file_info, i );
menu_font.setXY( (menu_font.num_xy[0] - (strlen( save_item_name ) /
2 + 15) ) / 2 );
if ( save_file_info.valid ){
sprintf( buffer, MESSAGE_SAVE_EXIST,
save_item_name,
save_file_info.sjis_no,
save_file_info.sjis_month,
save_file_info.sjis_day,
save_file_info.sjis_hour,
save_file_info.sjis_minute );
nofile_flag = false;
MESSAGE_SAVE_EXIST is 21 characters, not counting formatting strings and the
NUL terminator. Each of the sjis_* fields may be up to four characters,
making for 41, not 30 characters.
I have observed this behavior causing crashes in copies of onscripter that I
have built myself; I've also reported it to upstream (with a patch) but
it has evidently not been applied.
My patch is as follows; it's a bit brute force but allows enough space
for the sprintf to be safe:
---
onscripter-insani_20060724/build-tree/onscripter-20060724-insani/ONScripterLabel_rmenu.cpp
2006-06-22 00:16:52.0 -0400
+++
onscripter-insani_20060724.new/build-tree/onscripter-20060724-insani/ONScripterLabel_rmenu.cpp
2006-11-19 01:16:15.0 -0500
@@ -319,7 +319,7 @@
flush( refreshMode() );
bool nofile_flag;
-char *buffer = new char[ strlen( save_item_name ) + 30 + 1 ];
+char *buffer = new char[ strlen( save_item_name ) + 256 ];
for ( unsigned int i=1 ; i=num_save_file ; i++ ){
searchSaveFile( save_file_info, i );
@@ -401,7 +401,7 @@
flush( refreshMode() );
bool nofile_flag;
-char *buffer = new char[ strlen( save_item_name ) + 30 + 1 ];
+char *buffer = new char[ strlen( save_item_name ) + 256 ];
for ( unsigned int i=1 ; i=num_save_file ; i++ ){
SaveFileInfo save_file_info;
-- System Information:
Debian Release: lenny/sid
APT prefers unstable
APT policy: (500, 'unstable'), (500, 'testing'), (1, 'experimental')
Architecture: i386 (i686)
Kernel: Linux 2.6.21-2-k7 (SMP w/1 CPU core)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
