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
(<https://sourceforge.net/p/flightgear/mailman/message/35548661/> and
<http://lists.alioth.debian.org/pipermail/pkg-fgfs-crew/2016-December/001795.html>),
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
<https://www.debian.org/security/faq#discover>.
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
(<https://sourceforge.net/p/flightgear/flightgear/ci/280cd523686fbdb175d50417266d2487a8ce67d2/>)
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 <f.rou...@free.fr>
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 <simgear/misc/sg_path.hxx>
#include <simgear/sg_inlines.h>
+#include <Main/globals.hxx>
#include "Main/fg_props.hxx"
#include "Navaids/positioned.hxx"
#include <Navaids/waypoint.hxx>
@@ -55,6 +56,8 @@
#include "Airports/runways.hxx"
#include <GUI/new_gui.hxx>
#include <GUI/dialog.hxx>
+#include <Main/util.hxx> // fgValidatePath()
+#include <GUI/MessageBox.hxx>
#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.000000000 +0100
+++ flightgear-3.0.0/debian/changelog 2016-12-13 12:40:51.000000000 +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 <f.rou...@free.fr> 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 flightgear-3.0.0/debian/patches/fix-missing-lX11-in-link-commands.patch flightgear-3.0.0/debian/patches/fix-missing-lX11-in-link-commands.patch
--- flightgear-3.0.0/debian/patches/fix-missing-lX11-in-link-commands.patch 1970-01-01 01:00:00.000000000 +0100
+++ flightgear-3.0.0/debian/patches/fix-missing-lX11-in-link-commands.patch 2016-12-13 12:39:49.000000000 +0100
@@ -0,0 +1,26 @@
+Description: Fix build failures ('-lX11' missing for fgfs and fgviewer)
+ .
+ Tested in a jessie amd64 pbuilder chroot.
+Author: Florent Rougon <f.rou...@free.fr>
+Forwarded: not-needed
+
+--- a/utils/fgviewer/CMakeLists.txt
++++ b/utils/fgviewer/CMakeLists.txt
+@@ -48,5 +48,6 @@
+ ${OPENGL_LIBRARIES}
+ ${FGVIEWER_RTI_LIBRARIES}
+ ${SIMGEAR_CORE_LIBRARY_DEPENDENCIES}
++ ${PLATFORM_LIBS}
+ )
+ install(TARGETS fgviewer RUNTIME DESTINATION bin)
+--- a/CMakeLists.txt
++++ b/CMakeLists.txt
+@@ -127,6 +127,8 @@
+ if(GSM_FOUND)
+ set(SYSTEM_GSM_DEFAULT 1)
+ endif(GSM_FOUND)
++
++ list(APPEND PLATFORM_LIBS X11)
+ endif()
+
+ find_package(Git)
diff -Nru flightgear-3.0.0/debian/patches/route-manager-secu-fix-280cd5.patch flightgear-3.0.0/debian/patches/route-manager-secu-fix-280cd5.patch
--- flightgear-3.0.0/debian/patches/route-manager-secu-fix-280cd5.patch 1970-01-01 01:00:00.000000000 +0100
+++ flightgear-3.0.0/debian/patches/route-manager-secu-fix-280cd5.patch 2016-12-13 12:39:33.000000000 +0100
@@ -0,0 +1,52 @@
+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 <f.rou...@free.fr>
+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 <simgear/misc/sg_path.hxx>
+ #include <simgear/sg_inlines.h>
+
++#include <Main/globals.hxx>
+ #include "Main/fg_props.hxx"
+ #include "Navaids/positioned.hxx"
+ #include <Navaids/waypoint.hxx>
+@@ -55,6 +56,8 @@
+ #include "Airports/runways.hxx"
+ #include <GUI/new_gui.hxx>
+ #include <GUI/dialog.hxx>
++#include <Main/util.hxx> // fgValidatePath()
++#include <GUI/MessageBox.hxx>
+
+ #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/patches/series flightgear-3.0.0/debian/patches/series
--- flightgear-3.0.0/debian/patches/series 2015-03-18 08:48:58.000000000 +0100
+++ flightgear-3.0.0/debian/patches/series 2016-12-13 11:19:25.000000000 +0100
@@ -3,3 +3,5 @@
fix-mobile-tacan.patch
750939.patch
6a30e7.patch
+route-manager-secu-fix-280cd5.patch
+fix-missing-lX11-in-link-commands.patch
