torcs-devel Mailing List for TORCS - The Open Racing Car Simulator

Hi Bernhard,

in

if ( sscanf ( s,  "%s rgb %f %f %f amb %f %f %f emis %f %f %f spec %f %f %f
shi %d trans %f",

yeah I think It would be enough to put %1023s.

on the other hand in

ulSetError ( UL_WARNING, "grloadac:do_material: Can't parse this
MATERIAL:%1023s", s ) ;

remember, what you are sending to ulSetError is not sizeof(s) bytes, you
are actually sending
sizeof("grloadac:do_material: Can't parse this MATERIAL:")+sizeof(s), and
because plib's error buffer just
can store 1024 bytes, thats the reason the vulnerability is trigered. So
you have to chek how many bytes you send to ulSetError.

I sent an email to plib people, and they did not respond yet, but the last
message posted there in the mailing list is:

"PLIB is no longer being maintained - it would take too much work to bring
it up to the latest OpenGL standards.

I don't plan any more releases.  I recommend that FlightGear pulls their
current version into their repository and make it a part of FlightGear
itself.

 -- Steve"

So I dont know whether they are likely to fix this bug.
Anyway to solve PLIB bug It only would take to use  vsnprintf instead of
vsprintf inside ulSetError


Regards

2011/12/22 Bernhard Wymann <be...@bl...>
>
> Hi Andres
>
> So the fix would be then 2 parts:
>
>
>> in grloadacc.cpp
>>
>> static int do_material ( char *s )
>> {
>>   char name [ 1024 ] ;
>>   sgVec4 rgb  ;
>>   sgVec4 amb  ;
>>   sgVec4 emis ;
>>   sgVec4 spec ;
>>   int   shi ;
>>   float trans ;
>>
>>   if ( sscanf ( s,
>> "%s rgb %f %f %f amb %f %f %f emis %f %f %f spec %f %f %f shi %d trans
%f",
>
>
> So this should be then "%1023s ..., or less than 1023, right?
>
>
>>     name,
>> &rgb [0], &rgb [1], &rgb [2],
>> &amb [0], &amb [1], &amb [2],
>> &emis[0], &emis[1], &emis[2],
>> &spec[0], &spec[1], &spec[2],
>> &shi,
>> &trans ) != 15 )
>>   {
>
>
> Here we leave the context, so the right place to fix this is plib. But we
could also tune the format string (maybe we can get the buffer size
somehow, but for getting the idea):
>
> ulSetError ( UL_WARNING, "grloadac:do_material: Can't parse this
MATERIAL:%1023s", s ) ;
>
> Can you check if this does the trick? Did you already let the plib people
know?
>
> Thank you very much for your report and help
>
> Best regards
>
> Bernhard

Hi Andres

So the fix would be then 2 parts:

> in grloadacc.cpp
>
> static int do_material ( char *s )
> {
>    char name [ 1024 ] ;
>    sgVec4 rgb  ;
>    sgVec4 amb  ;
>    sgVec4 emis ;
>    sgVec4 spec ;
>    int   shi ;
>    float trans ;
>
>    if ( sscanf ( s,
> "%s rgb %f %f %f amb %f %f %f emis %f %f %f spec %f %f %f shi %d trans %f",

So this should be then "%1023s ..., or less than 1023, right?

>      name,
> &rgb [0], &rgb [1], &rgb [2],
> &amb [0], &amb [1], &amb [2],
> &emis[0], &emis[1], &emis[2],
> &spec[0], &spec[1], &spec[2],
> &shi,
> &trans ) != 15 )
>    {

Here we leave the context, so the right place to fix this is plib. But 
we could also tune the format string (maybe we can get the buffer size 
somehow, but for getting the idea):

ulSetError ( UL_WARNING, "grloadac:do_material: Can't parse this 
MATERIAL:%1023s", s ) ;

Can you check if this does the trick? Did you already let the plib 
people know?

Thank you very much for your report and help

Best regards

Bernhard

Ok, here are the details

the problem is that TORCS sometimes doesn't validate correctly data from
acc files. for example in:

car5-trb1.acc

AC3Db
MATERIAL "ac3dmat1" rgb 1.00 1.00 1.00 amb 1.00 1.00 1.00 emis 0.20 0.20
0.20 spec 0.50 0.50 0.50 shi 50 trans 0
OBJECT world
kids 48
OBJECT poly
name "DIFFUSORFLA_s_23"

if an attacker writes an very long string in MATERIAL line for example,

MATERIAL "aaaaaaaaaaaaaaaaaaaaaaaaaaa......aaaaaaaaaaaaaaaaaaaa" rgb 1.00
1.00 1.00 amb 1.00 1.00 1.00 emis 0.20 0.20 0.20 spec 0.50 0.50 0.50 shi 50
trans 0

then it will happen:

in grloadacc.cpp

static int do_material ( char *s )
{
  char name [ 1024 ] ;
  sgVec4 rgb  ;
  sgVec4 amb  ;
  sgVec4 emis ;
  sgVec4 spec ;
  int   shi ;
  float trans ;

  if ( sscanf ( s,
  "%s rgb %f %f %f amb %f %f %f emis %f %f %f spec %f %f %f shi %d trans
%f",
    name,
    &rgb [0], &rgb [1], &rgb [2],
    &amb [0], &amb [1], &amb [2],
    &emis[0], &emis[1], &emis[2],
    &spec[0], &spec[1], &spec[2],
    &shi,
    &trans ) != 15 )
  {
    ulSetError ( UL_WARNING, "grloadac:do_material: Can't parse this
MATERIAL:%s", s ) ;
  }

the line

if ( sscanf ( s,....

will fail and the function from plib ulSetError is gonna be called, with
the long string "s". Then in:

ulError.cpp from plib

void ulSetError ( enum ulSeverity severity, const char *fmt, ... )
{
  va_list argp;
  va_start ( argp, fmt ) ;
  vsprintf ( _ulErrorBuffer, fmt, argp ) ;
  va_end ( argp ) ;

  if ( _ulErrorCB )
  {
    (*_ulErrorCB)( severity, _ulErrorBuffer ) ;
  }
  else
  {
    fprintf ( stderr, "%s: %s\n",
       _ulSeverityText[ severity ], _ulErrorBuffer ) ;
    if ( severity == UL_FATAL )
    {
#ifdef WIN32
      // A Windows user that does not start the program from the command
line
      // will not see output to stderr
      ::MessageBox(0, _ulErrorBuffer, "fatal error!:", 0);
#endif
      exit (1) ;
    }
  }
}

this function is gonna overflow the buffer:

static char            _ulErrorBuffer [ 1024 ] = { '\0' } ;

which cause that pointer function

static ulErrorCallback _ulErrorCB = 0 ;

is overwritten and finally, that function pointer is called in:

if ( _ulErrorCB )
  {
    (*_ulErrorCB)( severity, _ulErrorBuffer ) ;
  }

An attacker can manipulate such string s, causing that torcs executes
arbitrary code. It's important to note that the main problem is in plib,
because it doesn't check the size of const char *fmt, for this reason the
bug is trigered, but TORCS also should check acc inputs.

Well it's true that this bug is not about remote arbitrary code excecution
but It should no be understimated. An attacker could convince a user to
open a specially crafted acc file (a new car, track, ...), and it would be
all he need to get total control over user's machine.

Regards, a any question is welcome.

2011/12/20 Bernhard Wymann <be...@bl...>

> Hi Andres
>
> I think it is ok to publish it in the list, because TORCS is not a
> networking/priviledged application, so there should (!) be nothing
> "exploitable" (if a user can run code and can run override/inject code in
> his own context it is a nice "playground", but not much more, it just gets
> interesting when this can happen from "outside"/other user, is this the
> case?). But it should be addressed for later network versions, where this
> could really hurt.
>
> Thank you for the info.
>
> Best regards
>
> Bernhard
>
> On Dec 20, 2011, at 16:04 , Andres Gomez wrote:
>
> > Hi,
> >
> > I have found an exploitable buffer overflow in torcs when it uses the
> library plib, I dont know whether I should disclose details in this mailing
> list or to another private mail.
> >
> >
> > Regards,
> > Andrés Gómez
> >
> >
> ------------------------------------------------------------------------------
> > Write once. Port to many.
> > Get the SDK and tools to simplify cross-platform app development. Create
> > new or port existing apps to sell to consumers worldwide. Explore the
> > Intel AppUpSM program developer opportunity. appdeveloper.intel.com/join
> >
> http://p.sf.net/sfu/intel-appdev_______________________________________________
> > Torcs-devel mailing list
> > Tor...@li...
> > https://lists.sourceforge.net/lists/listinfo/torcs-devel
>
>


-- 
Andrés Gómez Ramírez | Analista de Diagnóstico
Fluidsignal Group S.A. | Where Security Meets Business
http://www.fluidsignal.com/ | ISO 9001:2008 / ISO 27001:2005
Teléfono: +57 (4) 4442637 | Móvil: +57 3012009712

Hi Andres

I think it is ok to publish it in the list, because TORCS is not a networking/priviledged application, so there should (!) be nothing "exploitable" (if a user can run code and can run override/inject code in his own context it is a nice "playground", but not much more, it just gets interesting when this can happen from "outside"/other user, is this the case?). But it should be addressed for later network versions, where this could really hurt.

Thank you for the info.

Best regards

Bernhard

On Dec 20, 2011, at 16:04 , Andres Gomez wrote:

> Hi,
> 
> I have found an exploitable buffer overflow in torcs when it uses the library plib, I dont know whether I should disclose details in this mailing list or to another private mail.
> 
> 
> Regards,
> Andrés Gómez
> 
> ------------------------------------------------------------------------------
> Write once. Port to many.
> Get the SDK and tools to simplify cross-platform app development. Create 
> new or port existing apps to sell to consumers worldwide. Explore the 
> Intel AppUpSM program developer opportunity. appdeveloper.intel.com/join
> http://p.sf.net/sfu/intel-appdev_______________________________________________
> Torcs-devel mailing list
> Tor...@li...
> https://lists.sourceforge.net/lists/listinfo/torcs-devel

Hi,

I have found an exploitable buffer overflow in torcs when it uses the
library plib, I dont know whether I should disclose details in this mailing
list or to another private mail.


Regards,
Andrés Gómez