On Sat, Nov 03, 2012 at 02:54:01PM -0400, Michael Gilbert wrote:
> control: severity -1 serious
>
> Marking this as serious since it is somewhat common for users to
> download untrusted models, which could contain this malicious data.
Patches from Tom Callaway are attached.
Cheers,
Moritz
diff -up flightgear-2.6.0/src/Cockpit/panel.cxx.checkforn flightgear-2.6.0/src/Cockpit/panel.cxx
--- flightgear-2.6.0/src/Cockpit/panel.cxx.checkforn 2012-02-17 17:41:14.704313333 -0500
+++ flightgear-2.6.0/src/Cockpit/panel.cxx 2012-05-29 21:01:31.264831372 -0400
@@ -1209,8 +1209,18 @@ FGTextLayer::Chunk::Chunk (const string
: _type(FGTextLayer::TEXT), _fmt(fmt)
{
_text = text;
- if (_fmt.empty())
- _fmt = "%s";
+ if (_fmt.empty()) {
+ _fmt = "%s";
+ } else {
+ // It is never safe for _fmt.c_str to be %n.
+ string unsafe ("%n");
+ size_t found;
+ found=_fmt.find(unsafe);
+ if (found!=string::npos) {
+ SG_LOG(SG_COCKPIT, SG_WARN, "format type contained %n, but this is unsafe, reverting to %s");
+ _fmt = "%s";
+ }
+ }
}
FGTextLayer::Chunk::Chunk (ChunkType type, const SGPropertyNode * node,
@@ -1223,6 +1233,20 @@ FGTextLayer::Chunk::Chunk (ChunkType typ
_fmt = "%s";
else
_fmt = "%.2f";
+ } else {
+ // It is never safe for _fmt.c_str to be %n.
+ string unsafe ("%n");
+ size_t found;
+ found=_fmt.find(unsafe);
+ if (found!=string::npos) {
+ if (type == TEXT_VALUE) {
+ SG_LOG(SG_COCKPIT, SG_WARN, "format type contained %n, but this is unsafe, reverting to %s");
+ _fmt = "%s";
+ } else {
+ SG_LOG(SG_COCKPIT, SG_WARN, "format type contained %n, but this is unsafe, reverting to %.2f");
+ _fmt = "%.2f";
+ }
+ }
}
_node = node;
}
diff -up flightgear-2.6.0/src/Network/generic.cxx.checkforn flightgear-2.6.0/src/Network/generic.cxx
--- flightgear-2.6.0/src/Network/generic.cxx.checkforn 2012-02-17 17:41:16.428329558 -0500
+++ flightgear-2.6.0/src/Network/generic.cxx 2012-05-29 20:50:37.212255822 -0400
@@ -205,6 +205,8 @@ bool FGGeneric::gen_message_binary() {
bool FGGeneric::gen_message_ascii() {
string generic_sentence;
+ string unsafe ("%n");
+ size_t found;
char tmp[255];
length = 0;
@@ -215,6 +217,13 @@ bool FGGeneric::gen_message_ascii() {
generic_sentence += var_separator;
}
+ // It is never safe for _out_message[i].format.c_str to be %n.
+ found=_out_message[i].format.find(unsafe);
+ if (found!=string::npos) {
+ SG_LOG(SG_COCKPIT, SG_WARN, "format type contained %n, but this is unsafe, reverting to %s");
+ _out_message[i].format = "%s";
+ }
+
switch (_out_message[i].type) {
case FG_INT:
val = _out_message[i].offset +
diff -up flightgear-2.6.0/src/FDM/YASim/Rotor.cpp.rotornamemax256 flightgear-2.6.0/src/FDM/YASim/Rotor.cpp
--- flightgear-2.6.0/src/FDM/YASim/Rotor.cpp.rotornamemax256 2012-05-29 21:17:49.674896892 -0400
+++ flightgear-2.6.0/src/FDM/YASim/Rotor.cpp 2012-05-29 21:20:51.004474076 -0400
@@ -274,7 +274,7 @@ int Rotor::getValueforFGSet(int j,char *
if (4>numRotorparts()) return 0; //compile first!
if (j==0)
{
- sprintf(text,"/rotors/%s/cone-deg", _name);
+ snprintf(text, 256, "/rotors/%s/cone-deg", _name);
*f=(_balance1>-1)?( ((Rotorpart*)getRotorpart(0))->getrealAlpha()
+((Rotorpart*)getRotorpart(1*(_number_of_parts>>2)))->getrealAlpha()
+((Rotorpart*)getRotorpart(2*(_number_of_parts>>2)))->getrealAlpha()
@@ -284,7 +284,7 @@ int Rotor::getValueforFGSet(int j,char *
else
if (j==1)
{
- sprintf(text,"/rotors/%s/roll-deg", _name);
+ snprintf(text, 256, "/rotors/%s/roll-deg", _name);
_roll = ( ((Rotorpart*)getRotorpart(0))->getrealAlpha()
-((Rotorpart*)getRotorpart(2*(_number_of_parts>>2)))->getrealAlpha()
)/2*(_ccw?-1:1);
@@ -293,7 +293,7 @@ int Rotor::getValueforFGSet(int j,char *
else
if (j==2)
{
- sprintf(text,"/rotors/%s/yaw-deg", _name);
+ snprintf(text, 256, "/rotors/%s/yaw-deg", _name);
_yaw=( ((Rotorpart*)getRotorpart(1*(_number_of_parts>>2)))->getrealAlpha()
-((Rotorpart*)getRotorpart(3*(_number_of_parts>>2)))->getrealAlpha()
)/2;
@@ -302,38 +302,38 @@ int Rotor::getValueforFGSet(int j,char *
else
if (j==3)
{
- sprintf(text,"/rotors/%s/rpm", _name);
+ snprintf(text, 256, "/rotors/%s/rpm", _name);
*f=(_balance1>-1)?_omega/2/pi*60:0;
}
else
if (j==4)
{
- sprintf(text,"/rotors/%s/tilt/pitch-deg",_name);
+ snprintf(text, 256, "/rotors/%s/tilt/pitch-deg",_name);
*f=_tilt_pitch*180/pi;
}
else if (j==5)
{
- sprintf(text,"/rotors/%s/tilt/roll-deg",_name);
+ snprintf(text, 256, "/rotors/%s/tilt/roll-deg",_name);
*f=_tilt_roll*180/pi;
}
else if (j==6)
{
- sprintf(text,"/rotors/%s/tilt/yaw-deg",_name);
+ snprintf(text, 256, "/rotors/%s/tilt/yaw-deg",_name);
*f=_tilt_yaw*180/pi;
}
else if (j==7)
{
- sprintf(text,"/rotors/%s/balance", _name);
+ snprintf(text, 256, "/rotors/%s/balance", _name);
*f=_balance1;
}
else if (j==8)
{
- sprintf(text,"/rotors/%s/stall",_name);
+ snprintf(text, 256, "/rotors/%s/stall",_name);
*f=getOverallStall();
}
else if (j==9)
{
- sprintf(text,"/rotors/%s/torque",_name);
+ snprintf(text, 256, "/rotors/%s/torque",_name);
*f=-_torque;;
}
else
@@ -344,7 +344,7 @@ int Rotor::getValueforFGSet(int j,char *
return 0;
}
int w=j%3;
- sprintf(text,"/rotors/%s/blade[%i]/%s",
+ snprintf(text, 256, "/rotors/%s/blade[%i]/%s",
_name,b,
w==0?"position-deg":(w==1?"flap-deg":"incidence-deg"));
*f=((Rotorpart*)getRotorpart(0))->getPhi()*180/pi
