Re: TORCS + PLIB Buffer overflow detected

Andres Gomez <[email protected]> Thu, 22 Dec 2011 22:08:02 -0500
Newsgroups gmane.games.torcs.devel
Message-ID <CAB9ZNAyR6h9M7c=Com_HRPGirGD4LhrGWD5CuRN-bqs4kSYpyw@mail.gmail.com>
--===============0420927696252719423==
Content-Type: multipart/alternative; boundary=14dae9398f5bd3149a04b4b9baaa

--14dae9398f5bd3149a04b4b9baaa
Content-Type: text/plain; charset=ISO-8859-1

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 <[email protected]>
>
> 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

--14dae9398f5bd3149a04b4b9baaa
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

Hi Bernhard,<br><br>in<br><br> if ( sscanf ( s,=A0 &quot;%s rgb %f %f %f am=
b %f %f %f emis %f %f %f spec %f %f %f shi %d trans %f&quot;,<br><br>yeah I=
 think It would be enough to put %1023s.<br><br>on the other hand in<br><br=
>

ulSetError ( UL_WARNING, &quot;grloadac:do_material: Can&#39;t parse this M=
ATERIAL:%1023s&quot;, s ) ;<br><br>remember, what you are sending to ulSetE=
rror is not sizeof(s) bytes, you are actually sending<br>sizeof(&quot;grloa=
dac:do_material: Can&#39;t parse this MATERIAL:&quot;)+sizeof(s), and becau=
se plib&#39;s error buffer just <br>

can store 1024 bytes, thats the reason the vulnerability is trigered. So yo=
u have to chek how many bytes you send to ulSetError.<br><br>I sent an emai=
l to plib people, and they did not respond yet, but the last message posted=
 there in the mailing list is:<br>

<br>&quot;PLIB is no longer being maintained - it would take too much work =
to bring<br>
it up to the latest OpenGL standards.<br>
<br>
I don&#39;t plan any more releases. =A0I recommend that FlightGear pulls th=
eir<br>
current version into their repository and make it a part of FlightGear<br>
itself.<br>
<br>
 =A0-- Steve&quot;<br><br>So I dont know whether they are likely to fix thi=
s bug. <br>Anyway to solve PLIB bug It only would take to use=A0 vsnprintf =
instead of vsprintf inside ulSetError<br><br><br>Regards<br><br>2011/12/22 =
Bernhard Wymann &lt;<a href=3D"mailto:[email protected]">[email protected]<=
/a>&gt;<br>

&gt;<br>&gt; Hi Andres<br>&gt;<br>&gt; So the fix would be then 2 parts:<br=
>&gt;<br>&gt;<br>&gt;&gt; in grloadacc.cpp<br>&gt;&gt;<br>&gt;&gt; static i=
nt do_material ( char *s )<br>&gt;&gt; {<br>&gt;&gt; =A0 char name [ 1024 ]=
 ;<br>

&gt;&gt; =A0 sgVec4 rgb =A0;<br>&gt;&gt; =A0 sgVec4 amb =A0;<br>&gt;&gt; =
=A0 sgVec4 emis ;<br>&gt;&gt; =A0 sgVec4 spec ;<br>&gt;&gt; =A0 int =A0 shi=
 ;<br>&gt;&gt; =A0 float trans ;<br>&gt;&gt;<br>&gt;&gt; =A0 if ( sscanf ( =
s,<br>&gt;&gt; &quot;%s rgb %f %f %f amb %f %f %f emis %f %f %f spec %f %f =
%f shi %d trans %f&quot;,<br>

&gt;<br>&gt;<br>&gt; So this should be then &quot;%1023s ..., or less than =
1023, right?<br>&gt;<br>&gt;<br>&gt;&gt; =A0 =A0 name,<br>&gt;&gt; &amp;rgb=
 [0], &amp;rgb [1], &amp;rgb [2],<br>&gt;&gt; &amp;amb [0], &amp;amb [1], &=
amp;amb [2],<br>

&gt;&gt; &amp;emis[0], &amp;emis[1], &amp;emis[2],<br>&gt;&gt; &amp;spec[0]=
, &amp;spec[1], &amp;spec[2],<br>&gt;&gt; &amp;shi,<br>&gt;&gt; &amp;trans =
) !=3D 15 )<br>&gt;&gt; =A0 {<br>&gt;<br>&gt;<br>&gt; Here we leave the con=
text, so the right place to fix this is plib. But we could also tune the fo=
rmat string (maybe we can get the buffer size somehow, but for getting the =
idea):<br>

&gt;<br>&gt; ulSetError ( UL_WARNING, &quot;grloadac:do_material: Can&#39;t=
 parse this MATERIAL:%1023s&quot;, s ) ;<br>&gt;<br>&gt; Can you check if t=
his does the trick? Did you already let the plib people know?<br>&gt;<br>

&gt; Thank you very much for your report and help<br>&gt;<br>&gt; Best rega=
rds<br>&gt;<br>&gt; Bernhard<br><br>

--14dae9398f5bd3149a04b4b9baaa--


--===============0420927696252719423==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

------------------------------------------------------------------------------
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
--===============0420927696252719423==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
Torcs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/torcs-devel

--===============0420927696252719423==--