Package: lbreakout2
Severity: grave
Tags: security patch
Justification: user security hole
[ Details are sparse about this one, if you come to the conclusion that
this is not RC, then please lower severity, but I assume it is. ]
[ Woody might be affected as well, I haven't checked that yet ]
>From the 2.6beta changelog:
- security issue fixed: bad sprintf/scanf calls could lead to crashes
or potential security problems (05/02/14 U.H.)
There is no further information and upstream's website is very sparse
and there's no mailing list or anything like that, so I had a look
at the rather largish diff between 2.5.2 and 2.6beta and reviewed it
for possibly security relevant changes, which you can find attached.
At least a buffer overflow in highscore handling (should be writable by
arbitrary users) and the missing format strings in network multiplayer
handling code seem exploitable as lbreakout runs as setgid games.
Please review the diff and upload a fixed package to t-p-u, if you agree.
Cheers,
Moritz
-- System Information:
Debian Release: 3.1
APT prefers unstable
APT policy: (500, 'unstable')
Architecture: i386 (i686)
Kernel: Linux 2.6.11
Locale: LANG=C, [EMAIL PROTECTED] (charmap=ISO-8859-15)
diff -Naur lbreakout2-2.5.2/client/chart.c lbreakout2-2.6beta/client/chart.c
--- lbreakout2-2.5.2/client/chart.c 2003-05-25 11:37:05.000000000 +0200
+++ lbreakout2-2.5.2/client/chart.c 2005-03-28 20:07:30.000000000 +0200
@@ -138,7 +138,7 @@
if ( aux[0] != '>' ) break;
chart = calloc( 1, sizeof( Set_Chart ) );
/* get name: >>>name */
- fscanf( file, ">>>%s\n", setname );
+ fscanf( file, ">>>%1023s\n", setname );
chart->name = strdup( setname );
/* entries */
chart_read_entries( file, file_name, chart );
@@ -234,7 +234,7 @@
/* open file */
file = fopen( file_name, "w" );
if ( !file ) {
- fprintf( stderr, "??? Highscore chart loaded properly but cannot save?\n" );
+ fprintf( stderr, "??? Highscore chart loaded properly but cannot save? (%s)\n",file_name );
return;
}
/* save all charts */
diff -Naur lbreakout2-2.5.2/client/client_handlers.c lbreakout2-2.6beta/client/client_handlers.c
--- lbreakout2-2.5.2/client/client_handlers.c 2003-06-18 10:46:13.000000000 +0200
+++ lbreakout2-2.5.2/client/client_handlers.c 2005-02-16 22:07:28.000000000 +0100
@@ -146,7 +146,7 @@
/* extract ip and port and build a new socket out of it */
gui_edit_get_text( edit_server, server, 128, 0, -1 );
- snprintf( config.server, 64, server );
+ snprintf( config.server, 64, "%s", server );
if ( !net_build_addr( &newaddr, server, 0 ) ) {
client_printf_chatter( 1, "ERROR: address %s does not resolve", config.server );
return;
diff -Naur lbreakout2-2.5.2/client/client_recv.c lbreakout2-2.6beta/client/client_recv.c
--- lbreakout2-2.5.2/client/client_recv.c 2003-05-29 21:05:39.000000000 +0200
+++ lbreakout2-2.5.2/client/client_recv.c 2005-02-16 22:07:32.000000000 +0100
@@ -157,7 +157,7 @@
/* users */
case MSG_ADD_USER:
num = msg_read_int32();
- snprintf( name, 16, msg_read_string() ); name[15] = 0;
+ snprintf( name, 16, "%s", msg_read_string() ); name[15] = 0;
if ( msg_read_failed() ) break;
client_add_user( num, name );
gui_list_update(
@@ -223,8 +223,8 @@
client_transmit( CODE_BLUE, msglen, msgbuf );
break;
}
- snprintf( mp_peer_name, 15, msg_read_string() );
- snprintf( mp_levelset, 16, msg_read_string() );
+ snprintf( mp_peer_name, 15, "%s", msg_read_string() );
+ snprintf( mp_levelset, 16, "%s", msg_read_string() );
mp_diff = msg_read_int8();
mp_rounds = msg_read_int8();
mp_frags = msg_read_int8();
diff -Naur lbreakout2-2.5.2/client/comm.c lbreakout2-2.6beta/client/comm.c
--- lbreakout2-2.5.2/client/comm.c 2005-01-13 21:02:27.000000000 +0100
+++ lbreakout2-2.5.2/client/comm.c 2005-03-28 16:04:37.000000000 +0200
@@ -237,7 +237,7 @@
break;
case MSG_ADD_USER:
i = msg_read_int32();
- snprintf( name, 16, msg_read_string() ); name[15] = 0;
+ snprintf( name, 16, "%s", msg_read_string() ); name[15] = 0;
if ( msg_read_failed() ) break;
client_add_user( i, name );
handled = 1;
diff -Naur lbreakout2-2.5.2/client/editor.c lbreakout2-2.6beta/client/editor.c
--- lbreakout2-2.5.2/client/editor.c 2003-05-18 19:22:11.000000000 +0200
+++ lbreakout2-2.5.2/client/editor.c 2005-02-16 22:07:28.000000000 +0100
@@ -639,12 +639,12 @@
strcpy( str, "" );
if ( edit_buttons[x][y] == BUTTON_EDIT_AUTHOR )
if ( enter_string( font, "Author's Name:", str, 24 ) ) {
- snprintf( edit_cur_level->author, 31, str );
+ snprintf( edit_cur_level->author, 31, "%s", str );
*full_update = 1;
}
if ( edit_buttons[x][y] == BUTTON_EDIT_NAME )
if ( enter_string( font, "Title:", str, 24 ) ) {
- snprintf( edit_cur_level->name, 31, str );
+ snprintf( edit_cur_level->name, 31, "%s", str );
*full_update = 1;
}
/* sel frame tile position */
diff -Naur lbreakout2-2.5.2/game/comm.c lbreakout2-2.6beta/game/comm.c
--- lbreakout2-2.5.2/game/comm.c 2005-01-13 20:51:36.000000000 +0100
+++ lbreakout2-2.6beta/game/comm.c 2005-04-08 22:27:44.000000000 +0200
@@ -494,8 +494,8 @@
{
char *ptr = msg + *pos;
- snprintf( ptr, 16, level->name ); ptr[15] = 0; ptr += 16;
- snprintf( ptr, 16, level->author); ptr[15] = 0; ptr += 16;
+ snprintf( ptr, 16, "%s", level->name ); ptr[15] = 0; ptr += 16;
+ snprintf( ptr, 16, "%s", level->author); ptr[15] = 0; ptr += 16;
memcpy( ptr, level->bricks, 252 ); ptr += 252;
memcpy( ptr, level->extras, 252 ); ptr += 252;
@@ -507,8 +507,8 @@
{
char *ptr = msg + *pos;
- snprintf( level->name, 16, ptr ); ptr += 16;
- snprintf( level->author, 16, ptr ); ptr += 16;
+ snprintf( level->name, 16, "%s", ptr ); ptr += 16;
+ snprintf( level->author, 16, "%s", ptr ); ptr += 16;
memcpy( level->bricks, ptr, 252 ); ptr += 252;
memcpy( level->extras, ptr, 252 ); ptr += 252;
diff -Naur lbreakout2-2.5.2/game/levels.c lbreakout2-2.6beta/game/levels.c
--- lbreakout2-2.5.2/game/levels.c 2004-09-25 19:41:06.000000000 +0200
+++ lbreakout2-2.5.2/game/levels.c 2005-05-15 13:39:51.000000000 +0200
@@ -74,7 +74,7 @@
if ( fname[0] != '/' ) /* keep global pathes */
snprintf( path, sizeof(path)-1, "%s/levels/%s", SRC_DIR, fname );
else
- snprintf( path, sizeof(path)-1, fname );
+ snprintf( path, sizeof(path)-1, "%s", fname );
if ( ( file = fopen( path, mode ) ) == 0 ) {
fprintf( stderr, "couldn't open %s\n", path );
@@ -192,7 +192,7 @@
if ( levels->count == 0 ) return 0;
set = salloc( 1, sizeof( LevelSet ) );
- snprintf( set->name, 20, name );
+ snprintf( set->name, 20, "%s", name );
set->levels = salloc( levels->count, sizeof( Level* ) );
set->count = levels->count;
set->version = version;
@@ -344,10 +344,10 @@
if ( !strequal( "Level:", buffer ) ) goto failure;
/* author */
if ( !next_line( file, buffer ) ) goto failure;
- snprintf( level->author, 31, buffer );
+ snprintf( level->author, 31, "%s", buffer );
/* level name */
if ( !next_line( file, buffer ) ) goto failure;
- snprintf( level->name, 31, buffer );
+ snprintf( level->name, 31, "%s", buffer );
/* bricks: */
if ( !next_line( file, buffer ) ) goto failure;
if ( !strequal( "Bricks:", buffer ) ) goto failure;
@@ -389,8 +389,8 @@
{
int i, j;
Level *level = calloc( 1, sizeof( Level ) );
- snprintf( level->author, 31, author );
- snprintf( level->name, 31, name );
+ snprintf( level->author, 31, "%s", author );
+ snprintf( level->name, 31, "%s", name );
/* empty arena */
for ( i = 0; i < EDIT_WIDTH; i++ )
for ( j = 0; j < EDIT_HEIGHT; j++ ) {
diff -Naur lbreakout2-2.5.2/gui/gui_edit.c lbreakout2-2.6beta/gui/gui_edit.c
--- lbreakout2-2.5.2/gui/gui_edit.c 2003-06-04 21:15:30.000000000 +0200
+++ lbreakout2-2.5.2/gui/gui_edit.c 2005-02-16 22:07:24.000000000 +0100
@@ -422,7 +422,7 @@
{
if ( widget->type != GUI_EDIT ) return;
/* copy text */
- snprintf( widget->spec.edit.buffer, widget->spec.edit.size + 1, text );
+ snprintf( widget->spec.edit.buffer, widget->spec.edit.size + 1, "%s", text );
widget->spec.edit.length = strlen( widget->spec.edit.buffer );
/* reset */
/* first character in first line */
@@ -456,7 +456,7 @@
if ( length > limit )
length = limit;
if ( length )
- snprintf( buffer, limit, widget->spec.edit.buffer );
+ snprintf( buffer, limit, "%s", widget->spec.edit.buffer );
else
buffer[0] = 0;
return 1;
