Hi Andres,
thanks for pointing these out. We have been chasing and replacing
(s)(n)printfs in our code over the years but not at a high priority.
Everytime I (and others) are working on a file and stumble upon a
printf, we try to replace this with more robust code.
This is low priority, because the possible code injection can only
happen by the user itself and usually not over the (inter)net. And
FlightGear is supposed to run in the user's context which should add
some extra safety. (Never run fgfs as root or Administrator!)
But anyway, these are bad coding styles and should be replaced, the
sooner the better.
Torsten
Am 20.03.2012 18:18, schrieb Andres Gomez:
> Hi Curtis,
>
> Here I send details about buffer overflows I commented before:
>
> The first one is in flightgear/src/FDM/YASim/Rotor.cpp
>
> line 271 int Rotor::getValueforFGSet(int j,char *text,float *f)
> {
> .
> .
> .
> line 277 sprintf(text,"/rotors/%s/cone-deg", _name);
> line 287 sprintf(text,"/rotors/%s/roll-deg", _name);
> .
> .
> .
> And every similar sprintf in the function getValueforFGSet which copies
> _name string inside text buffer. If rotor's name is very long for
> example in fgdata/Aircraft/bo105/bo105.xml:
>
> <rotor name="AAAAAAAAAAAAAAAAAA......AAAAAAAAAA" x="-2.75" y="0.0"
> z="1.55" nx="0.05" ny="0" nz="1." fx="1" fy="0" fz="0" ccw="1"
>
> then string "AAAAAAAAAAAAAAAAAA......AAAAAAAAAA" will overflow text
> buffer (256 bytes), which could allow local arbitrary code execution by
> a corrupted xml model with rotor tag.
>
> The second one is in simgear/simgear/simgear/io/sg_socket_udp.cxx:
>
> line 101 int SGSocketUDP::read( char *buf, int length ) {
> .
> .
> .
> line 108 if ( (result = sock.recv(buf, SG_IO_MAX_MSG_SIZE, 0)) >= 0 ) {
>
> If buf size is less than SG_IO_MAX_MSG_SIZE, then such buffer is going
> to be overflowed, which could allow even remote arbitrary code
> execution. However I'm wondering if that code section is being used for
> flightgear at all. It seems that It is not. Looking at google I found a
> project using that simgear code section:
>
> http://forge.scilab.org/index.php/p/aerospace-toolbox/
> forge.scilab.org/index.php/p/aerospace-toolbox/source/file/277c4d9fd4a7d23dedef34919c100c80c893e983/sci_gateway/cpp/sci_cpp_find.cxx
> <http://forge.scilab.org/index.php/p/aerospace-toolbox/source/file/277c4d9fd4a7d23dedef34919c100c80c893e983/sci_gateway/cpp/sci_cpp_find.cxx>
>
> Regards,
>
> Andres Gomez
>
> 2012/3/13 Curtis Olson <curtolson@... <mailto:curtolson@...>>
>
> Hi Andres,
>
> Yes I saw your email, but I'm traveling this week and away from my
> desktop computer and all of the code. I was hoping one of the other
> developers would pick this up and look at it ... otherwise remind me
> next week after I've had a few days to catch up from being gone.
>
> Regards,
>
> Curt.
>
> On Mar 13, 2012 9:57 AM, "Andres Gomez" <agomez@...
> <mailto:agomez@...>> wrote:
>
> **
>
> Hi Curtis,
>
> *Are you flightgear's project manager, right?
>
> Did you see last email I sent to devel list about several
> vulnerabilities in flightgeat? I had no answer so I'm writing
> you in case you didn't see it.
> By the way I recently have also found a couple of buffer overflows.
>
> Regards.
> *
> 2012/3/9 Andres Gomez <agomez@...
> <mailto:agomez@...>>
>
> Hi,
>
> I have found multiple format string vulnerabilities in
> Flightgear and Simgear. This could allow an attacker to
> execute arbitrary code in a Flightgear user's machine. This
> is possible because user controlled format string is passed
> directly to printf family functions without any validation.
> For example if I have an aircraft xml model with a section
> like this:
>
>
> <text>
> <name>Registration</name>
> <type type="string">text-value</type>
> <property type="string">/sim/multiplay/callsign</property>
> <format type="string">%s</format>
> <draw-text type="bool">true</draw-text>
> .
> .
> .
> </text>
>
> the format string "%s" in label
>
> <format type="string">%s</format>
>
> is passed directly to snprintf. This line can be changed for
> something like "%s %s %s %s" which will make Flightgear to
> crash. Even more if "%n" specifier//is used, arbitrary code
> execution can be achieved. Until now I have found this issue
> in the following files:
>
> fgfs/flightgear/src/Cockpit/panel.cxx:1237
> fgfs/flightgear/src/Cockpit/panel.cxx:1240
> fgfs/flightgear/src/Cockpit/panel.cxx:1245
> fgfs/flightgear/src/Network/generic.cxx:222
>
> simgear/simgear/scene/model/SGText.cxx:72
> simgear/simgear/scene/model/SGText.cxx:74
>
> but others locations could also be affected. A solution for
> this bug would be at least to validate that "n" specifier is
> not present in the format string.
>
> Regards,
>
> Andrés Gómez
>
>
>
> --
> AVISO DE CONFIDENCIALIDAD:
>
> Esta transmisión se entiende para uso del destinatario o la
> entidad a la que va dirigida y puede contener información
> confidencial o protegida por la ley. Si el lector de este
> mensaje no fuera el destinatario, considérese por este medio
> informado que la retención, difusión, o copia de este correo
> electrónico está estrictamente prohibida. Si recibe este mensaje
> por error, por favor notifique inmediatamente al emisor y
> destruya el original. Gracias
>
> --
> CONFIDENTIALITY NOTICE:
>
> This transmission is intended for the use of the individual or
> entity to which it is addressed, and it may contain information
> that is confidential or privileged under law. If the reader of
> this message is not the intended recipient, you are hereby
> notified that retention, dissemination, distribution or copying
> of this e-mail is strictly prohibited. If you received this
> e-mail in error, please notify the sender immediately and
> destroy the original. Thank you.
>
>
>
> --
> AVISO DE CONFIDENCIALIDAD:
>
> Esta transmisión se entiende para uso del destinatario o la entidad a la
> que va dirigida y puede contener información confidencial o protegida
> por la ley. Si el lector de este mensaje no fuera el destinatario,
> considérese por este medio informado que la retención, difusión, o copia
> de este correo electrónico está estrictamente prohibida. Si recibe este
> mensaje por error, por favor notifique inmediatamente al emisor y
> destruya el original. Gracias
>
> --
> CONFIDENTIALITY NOTICE:
>
> This transmission is intended for the use of the individual or entity to
> which it is addressed, and it may contain information that is
> confidential or privileged under law. If the reader of this message is
> not the intended recipient, you are hereby notified that retention,
> dissemination, distribution or copying of this e-mail is strictly
> prohibited. If you received this e-mail in error, please notify the
> sender immediately and destroy the original. Thank you.
>
>
> ------------------------------------------------------------------------------
> This SF email is sponsosred by:
> Try Windows Azure free for 90 days Click Here
> http://p.sf.net/sfu/sfd2d-msazure
>
>
>
> _______________________________________________
> Flightgear-devel mailing list
> Flightgear-devel@...
> https://lists.sourceforge.net/lists/listinfo/flightgear-devel
|