Bug#848114: flightgear: Allows the route manager to overwrite arbitrary files

[Florent Rougon]

Markus Wanner  wrote:

> Hello Florent,
>
> thanks a lot for your notification and the patch(es). Uploads to stable
> (security) and unstable should follow, shortly.

Fine, thank you, Markus!

Regards

-- 
Florent

Bug#848114: flightgear: Allows the route manager to overwrite arbitrary files

[Markus Wanner]

Control: tags -1 +pending

Hello Florent,

thanks a lot for your notification and the patch(es). Uploads to stable
(security) and unstable should follow, shortly.

Kind Regards

Markus Wanner




signature.asc
Description: OpenPGP digital signature

Bug#848114: flightgear: Allows the route manager to overwrite arbitrary files

[Florent Rougon]

Source: flightgear
Version: 3.0.0-5
Severity: grave
Tags: security upstream fixed-upstream patch
Justification: user security hole

Hello,

As already stated in several places:

  
https://sourceforge.net/p/flightgear/flightgear/ci/280cd523686fbdb175d50417266d2487a8ce67d2/
  https://sourceforge.net/p/flightgear/mailman/message/35548661/
  
http://lists.alioth.debian.org/pipermail/pkg-fgfs-crew/2016-December/001795.html

and reported to people in charge of FlightGear both upstream (of which I am a
recent addition) and in several Linux distributions, the flightgear package
has a security bug allowing malicious Nasal code[1] to overwrite arbitrary
files the user running FlightGear has write access to, by using the property
tree to cause the route manager to save a flightplan.

This problem is, AFAICT, present in all FlightGear versions released after
October 5, 2009, which largely includes those shipped in Debian stable,
testing and unstable. It is however fixed in the upstream Git repository:

  
https://sourceforge.net/p/flightgear/flightgear/ci/280cd523686fbdb175d50417266d2487a8ce67d2/

and I have backported this fix to FlightGear 3.0.0, i.e., the version shipped
in jessie: cf. two links given above
( and
),
the second one being more ready-to-use for Debian since it contains a debdiff
including an additional fix for build failures I encountered while testing the
fix in the jessie package.

Since all parties have already been contacted, this bug report is mainly for
tracking purposes, as advised by
.

I'm attaching here the patch for FlightGear 3.0.0 as well as the mentioned
debdiff for completeness and “self-containedness” of this report. The upstream
fix
()
can certainly be used as is for the version in unstable.

Regards

[1] Which can be embedded in aircraft, which can in their turn be installed by
users from various third-party sources.
Description: Security fix: don't allow the route manager to overwrite arbitrary files
 Since the Save function of the route manager can be triggered from Nasal with
 an arbitrary path, we must check the path before overwriting the file.
 .
 (also add a missing include that is directly needed for this commit)
Author: Florent Rougon 
Origin: upstream, https://sourceforge.net/p/flightgear/flightgear/ci/280cd523686fbdb175d50417266d2487a8ce67d2/

--- a/src/Autopilot/route_mgr.cxx
+++ b/src/Autopilot/route_mgr.cxx
@@ -47,6 +47,7 @@
 #include 
 #include 
 
+#include 
 #include "Main/fg_props.hxx"
 #include "Navaids/positioned.hxx"
 #include 
@@ -55,6 +56,8 @@
 #include "Airports/runways.hxx"
 #include 
 #include 
+#include // fgValidatePath()
+#include 
 
 #define RM "/autopilot/route-manager/"
 
@@ -707,7 +710,23 @@ void FGRouteMgr::InputListener::valueChanged(SGPropertyNode *prop)
   mgr->loadRoute(path);
 } else if (!strcmp(s, "@SAVE")) {
   SGPath path(mgr->_pathNode->getStringValue());
-  mgr->saveRoute(path);
+  const std::string authorizedPath = fgValidatePath(path.str(),
+true /* write */);
+
+  if (!authorizedPath.empty()) {
+mgr->saveRoute(authorizedPath);
+  } else {
+const SGPath proposedPath = SGPath(globals->get_fg_home()) / "Export";
+std::string msg =
+  "The route manager was asked to write the flightplan to '" +
+  path.str() + "', but this path is not authorized for writing. " +
+  "Please choose another location, for instance in the $FG_HOME/Export "
+  "folder (" + proposedPath.str() + ").";
+
+SG_LOG(SG_AUTOPILOT, SG_ALERT, msg);
+modalMessageBox("FlightGear", "Unable to write to the specified file",
+msg);
+  }
 } else if (!strcmp(s, "@NEXT")) {
   mgr->jumpToIndex(mgr->currentIndex() + 1);
 } else if (!strcmp(s, "@PREVIOUS")) {
diff -Nru flightgear-3.0.0/debian/changelog flightgear-3.0.0/debian/changelog
--- flightgear-3.0.0/debian/changelog	2015-03-18 11:19:39.0 +0100
+++ flightgear-3.0.0/debian/changelog	2016-12-13 12:40:51.0 +0100
@@ -1,3 +1,13 @@
+flightgear (3.0.0-5+deb8u1) jessie; urgency=medium
+
+  * Add patch route-manager-secu-fix-280cd5.patch (security fix preventing
+the route manager from being able to overwrite arbitrary files
+writable by the user running FlightGear).
+  * Add patch fix-missing-lX11-in-link-commands.patch to fix an FTBFS
+failure due to -lX11 missing in two link commands.
+
+ -- Florent Rougon   Tue, 13 Dec 2016 12:40:51 +0100
+
 flightgear (3.0.0-5) unstable; urgency=high
 
   * Add patch 6a30e70.patch to better restrict file access from
diff -Nru