Add patch: 602_nx-X11_initgroups.full.patch. Fix calling setuid and setgid without setgroups or initgroups. There is a high probability this means it didn't relinquish all groups, and this would be a potential security issue to be fixed. Seek POS36-C on the web for details about the problem. (Fixes: #293).

3 files changed, 73 insertions, 0 deletions

diff --git a/debian/changelog b/debian/changelog
index 2242a75da..b523ee19b 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -19,6 +19,11 @@ nx-libs (2:3.5.0.21-0) UNRELEASED; urgency=low
   * Change build options so that bundled libraries are not used anymore at
     build time. Remove bundled libraries from rolled tarballs, as well. (Fixes:
     #238).
+  * Add patch: 602_nx-X11_initgroups.full.patch. Fix calling setuid and setgid
+    without setgroups or initgroups. There is a high probability this means it
+    didn't relinquish all groups, and this would be a potential security issue
+    to be fixed. Seek POS36-C on the web for details about the problem. (Fixes:
+    #293).
 
  -- Mike Gabriel <mike.gabriel@das-netzwerkteam.de>  Thu, 28 Mar 2013 21:07:42 +0100
 
diff --git a/debian/patches/602_nx-X11_initgroups.full.patch b/debian/patches/602_nx-X11_initgroups.full.patch
new file mode 100644
index 000000000..182b378dc
--- /dev/null
+++ b/debian/patches/602_nx-X11_initgroups.full.patch
@@ -0,0 +1,67 @@
+Description: Be compliant with POS36-C: Observe correct revocation order while relinquishing privileges
+Author: Orion Poplawski <orion@cora.nwra.com>
+Abstract:
+ The Fedora review of NX (redistributed) caught the following rpmlint issue:
+ .
+ This executable is calling setuid and setgid without setgroups or initgroups.
+ There is a high probability this mean it didn't relinquish all groups, and this
+ would be a potential security issue to be fixed. Seek POS36-C on the web for
+ details about the problem.
+ .
+ Ref POS36-C:
+ https://www.securecoding.cert.org/confluence/display/seccode/POS36-C.+Observe+correct+revocation+order+while+relinquishing+privileges
+ .
+ This patch adds initgroups() calls to the code to initialize the supplemental group list.
+diff --git a/nx-X11/programs/Xserver/os/utils.c b/nx-X11/programs/Xserver/os/utils.c
+index 7e62654..9b2431a 100644
+--- a/nx-X11/programs/Xserver/os/utils.c
++++ b/nx-X11/programs/Xserver/os/utils.c
+@@ -112,6 +112,9 @@ OR PERFORMANCE OF THIS SOFTWARE.
+ #include <sys/stat.h>
+ #include <ctype.h>    /* for isspace */
+ #include <stdarg.h>
++#include <sys/types.h>
++#include <grp.h>
++#include <pwd.h>
+ 
+ #if defined(DGUX)
+ #include <sys/resource.h>
+@@ -1770,6 +1773,7 @@ System(char *command)
+     void (*csig)(int);
+ #endif
+     int status;
++    struct passwd *pwent;
+ 
+     if (!command)
+ 	return(1);
+@@ -1791,6 +1795,9 @@ System(char *command)
+     case -1:	/* error */
+ 	p = -1;
+     case 0:	/* child */
++	pwent = getpwuid(getuid());
++	if (initgroups(pwent->pw_name,getgid()) == -1)
++	    _exit(127);
+ 	if (setgid(getgid()) == -1)
+ 	    _exit(127);
+ 	if (setuid(getuid()) == -1)
+diff --git a/nxcomp/Pipe.cpp b/nxcomp/Pipe.cpp
+index 7238d0c..aacbbae 100644
+--- a/nxcomp/Pipe.cpp
++++ b/nxcomp/Pipe.cpp
+@@ -21,6 +21,7 @@
+ #include <pwd.h>
+ #include <sys/types.h>
+ #include <sys/wait.h>
++#include <grp.h>
+ 
+ #include "Pipe.h"
+ #include "Misc.h"
+@@ -234,6 +235,8 @@ FILE *Popen(char * const parameters[], const char *type)
+       // Child.
+       //
+ 
++      struct passwd *pwent = getpwuid(getuid());
++      if (pwent) initgroups(pwent->pw_name,getgid());
+       setgid(getgid());
+       setuid(getuid());
+ 
diff --git a/debian/patches/series b/debian/patches/series
index 2d95bf5b1..8c5eebdc5 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -47,6 +47,7 @@
 302_nxagent_configurable-keystrokes.full.patch
 600_nx-X11+nxcompext+nxcompshad_unique-libnames.full.patch
 601_nx-X11_build-option-changes-to-not-use-bundled-libraries.full.patch
+602_nx-X11_initgroups.full.patch
 999_nxagent_unbrand-nxagent-brand-x2goagent.full.patch
 016_nx-X11_install-location.debian.patch
 102_xserver-xext_set-securitypolicy-path.debian.patch