fs/binfmt: close symlink TOCTOU and harden setuid/setgid exec hygiene

Perform pseudo-filesystem permission checks inside inode_reserve() and
inode_remove() while the inode tree lock is held, and hold that lock across
pseudorename mutations so symlink swaps cannot bypass directory checks.
Hold a read lock around pseudo-fs open permission checks.

On setuid/setgid exec, update saved set-IDs, mark the task group secure,
sanitize dangerous environment variables, clear debug/dumpable flags, and
add issetugid(), secure_getenv(), and PR_SET/GET_DUMPABLE support.

Signed-off-by: Abhishek Mishra <mishra.abhishek2808@gmail.com>

binfmt/binfmt_execmodule.c

@@ -45,6 +45,7 @@
 #include <nuttx/binfmt/binfmt.h>
 
 #include "binfmt.h"
+#include "environ/environ.h"
 
 #ifndef CONFIG_BINFMT_DISABLE
 
@@ -301,14 +302,29 @@ int exec_module(FAR struct binary_s *binp,
 #endif
 
 #ifdef CONFIG_SCHED_USER_IDENTITY
-  if (binp->mode & S_ISUID)
+  /* Apply set-user-ID / set-group-ID credentials for the new image. */
-    {
-      tcb->group->tg_euid = binp->uid;
-    }
 
-  if (binp->mode & S_ISGID)
+  if ((binp->mode & (S_ISUID | S_ISGID)) != 0)
     {
-      tcb->group->tg_egid = binp->gid;
+      FAR struct task_group_s *group = tcb->group;
+
+      if ((binp->mode & S_ISUID) != 0)
+        {
+          group->tg_euid = binp->uid;
+          group->tg_suid = binp->uid;
+        }
+
+      if ((binp->mode & S_ISGID) != 0)
+        {
+          group->tg_egid = binp->gid;
+          group->tg_sgid = binp->gid;
+        }
+
+      group->tg_flags |= GROUP_FLAG_SECURE_EXEC;
+      group->tg_flags &= ~(GROUP_FLAG_FD_BACKTRACE | GROUP_FLAG_DUMPABLE);
+#ifndef CONFIG_DISABLE_ENVIRON
+      env_sanitize_secure(group);
+#endif
     }
 #endif
 

fs/inode/fs_inoderemove.c

@@ -79,6 +79,18 @@ static FAR struct inode *inode_unlink(FAR const char *path)
       inode = desc.node;
       DEBUGASSERT(inode != NULL);
 
+#ifdef CONFIG_FS_PERMISSION
+      if (desc.parent != NULL)
+        {
+          ret = inode_checkperm(desc.parent, W_OK);
+          if (ret < 0)
+            {
+              inode = NULL;
+              goto errout;
+            }
+        }
+#endif
+
       /* If peer is non-null, then remove the node from the right of
        * of that peer node.
        */

fs/inode/fs_inodereserve.c

@@ -226,6 +226,17 @@ int inode_reserve(FAR const char *path,
   left   = desc.peer;
   parent = desc.parent;
 
+#ifdef CONFIG_FS_PERMISSION
+  if (parent != NULL)
+    {
+      ret = inode_checkperm(parent, W_OK | X_OK);
+      if (ret < 0)
+        {
+          goto errout_with_search;
+        }
+    }
+#endif
+
   for (; ; )
     {
       FAR struct inode *node;

fs/inode/inode.h

@@ -374,6 +374,7 @@ void inode_root_reserve(void);
  *
  *   EINVAL - 'path' is invalid for this operation
  *   EEXIST - An inode already exists at 'path'
+ *   EACCES - Caller lacks permission on the parent directory
  *   ENOMEM - Failed to allocate in-memory resources for the operation
  *
  ****************************************************************************/
@@ -390,6 +391,10 @@ int inode_reserve(FAR const char *path,
  *   inode is in-use, then it will be unlinked, but will not be freed until
  *   the last reference to the inode is released.
  *
+ * Returned Value:
+ *   Zero (OK) on success; a negated errno value on failure, including
+ *   -EACCES if the caller lacks write permission on the parent directory.
+ *
  * Assumptions/Limitations:
  *   The caller must hold the inode semaphore
  *

fs/vfs/fs_mkdir.c

@@ -137,19 +137,11 @@ int mkdir(const char *pathname, mode_t mode)
 
   else
     {
-      /* Verify write+search permission on the parent directory before
-       * adding a new name to the pseudo-filesystem tree.  POSIX requires
-       * both W_OK and X_OK to create a directory entry.
-       */
-
-      ret = inode_checkperm(desc.parent, W_OK | X_OK);
-      if (ret < 0)
-        {
-          errcode = -ret;
-          goto errout_with_search;
-        }
-
       /* Create an inode in the pseudo-filesystem at this path.
+       * inode_reserve() resolves the path and checks parent-directory
+       * permissions while the inode tree lock is held, closing symlink
+       * TOCTOU windows.
+       *
        * NOTE that the new inode will be created with a reference
        * count of zero.
        */

fs/vfs/fs_open.c

@@ -172,7 +172,19 @@ static int file_vopen(FAR struct file *filep, FAR const char *path,
 
   /* Validate operation support and pseudo-filesystem permissions */
 
-  ret = inode_checkopenperm(inode, oflags);
+#ifndef CONFIG_DISABLE_MOUNTPOINT
+  if (INODE_IS_MOUNTPT(inode))
+    {
+      ret = inode_checkopenperm(inode, oflags);
+    }
+  else
+#endif
+    {
+      inode_rlock();
+      ret = inode_checkopenperm(inode, oflags);
+      inode_runlock();
+    }
+
   if (ret < 0)
     {
       goto errout_with_inode;

fs/vfs/fs_rename.c

@@ -71,32 +71,34 @@ static int pseudorename(FAR const char *oldpath, FAR struct inode *oldinode,
                         FAR const char *newpath)
 {
   struct inode_search_s newdesc;
-  struct inode_search_s pardesc;
+  struct inode_search_s olddesc;
   FAR struct inode *newinode;
-  FAR struct inode *parnode;
   FAR char *subdir = NULL;
 #ifdef CONFIG_FS_NOTIFY
   bool isdir = INODE_IS_PSEUDODIR(oldinode);
 #endif
   int ret;
 
-  /* SETUP_SEARCH early so RELEASE_SEARCH at errout is safe. */
+  /* Hold the inode tree lock across resolve / permission checks / mutate
+   * so symbolic links cannot be swapped between check and use.
+   * Destination parent permissions are enforced by inode_reserve().
+   */
+
+  inode_lock();
 
   SETUP_SEARCH(&newdesc, newpath, true);
 
-  /* Verify source parent write permission. */
-
   ret = inode_checkperm(oldparent, W_OK);
   if (ret < 0)
     {
-      goto errout;
+      goto errout_with_lock;
     }
 
   /* According to POSIX, any new inode at this path should be removed
    * first, provided that it is not a directory.
    */
 
-  ret = inode_find(&newdesc);
+  ret = inode_search(&newdesc);
   if (ret >= 0)
     {
       /* We found it.  Get the search results */
@@ -110,9 +112,8 @@ static int pseudorename(FAR const char *oldpath, FAR struct inode *oldinode,
 
       if (oldinode == newinode)
         {
-          inode_release(newinode);
           ret = OK;
-          goto errout; /* Same name, this is not an error case. */
+          goto errout_with_lock;
         }
 
 #ifndef CONFIG_DISABLE_MOUNTPOINT
@@ -120,9 +121,8 @@ static int pseudorename(FAR const char *oldpath, FAR struct inode *oldinode,
 
       if (INODE_IS_MOUNTPT(newinode))
         {
-          inode_release(newinode);
           ret = -EXDEV;
-          goto errout;
+          goto errout_with_lock;
         }
 #endif
 
@@ -145,7 +145,7 @@ static int pseudorename(FAR const char *oldpath, FAR struct inode *oldinode,
             {
               subdir = NULL;
               ret = -ENOMEM;
-              goto errout;
+              goto errout_with_lock;
             }
 
           newpath = subdir;
@@ -162,37 +162,16 @@ static int pseudorename(FAR const char *oldpath, FAR struct inode *oldinode,
            * won't really be removed until we call inode_release();
            */
 
-          inode_remove(newpath);
+          ret = inode_remove(newpath);
+          if (ret < 0 && ret != -EBUSY)
+            {
+              goto errout_with_lock;
+            }
+
 #ifdef CONFIG_FS_NOTIFY
           notify_unlink(newpath);
 #endif
         }
-
-      inode_release(newinode);
-    }
-
-  /* Re-resolve the final destination parent after path rewrite. */
-
-  SETUP_SEARCH(&pardesc, newpath, true);
-  inode_find(&pardesc);   /* pardesc.parent valid even if node not found */
-  parnode = pardesc.node;
-
-  ret = inode_checkperm(pardesc.parent, W_OK);
-
-  /* inode_find() holds a reference on parnode; RELEASE_SEARCH() only
-   * frees pardesc.buffer.
-   */
-
-  if (parnode != NULL)
-    {
-      inode_release(parnode);
-    }
-
-  RELEASE_SEARCH(&pardesc);
-
-  if (ret < 0)
-    {
-      goto errout;
     }
 
   /* Create a new, empty inode at the destination location.
@@ -200,16 +179,21 @@ static int pseudorename(FAR const char *oldpath, FAR struct inode *oldinode,
    * of  zero.
    */
 
-  inode_lock();
   ret = inode_reserve(newpath, 0777, &newinode);
   if (ret < 0)
     {
-      /* It is an error if a node at newpath already exists in the tree
+      goto errout_with_lock;
-       * OR if we fail to allocate memory for the new inode (and possibly
+    }
-       * any new intermediate path segments).
-       */
 
-      ret = -EEXIST;
+  /* Re-resolve the source under the same lock before unlinking it. */
+
+  SETUP_SEARCH(&olddesc, oldpath, true);
+  ret = inode_search(&olddesc);
+  RELEASE_SEARCH(&olddesc);
+  if (ret < 0 || olddesc.node != oldinode)
+    {
+      inode_remove(newpath);
+      ret = -ENOENT;
       goto errout_with_lock;
     }
 
@@ -265,6 +249,7 @@ static int pseudorename(FAR const char *oldpath, FAR struct inode *oldinode,
   ret = OK;
 
 errout_with_lock:
+  RELEASE_SEARCH(&newdesc);
   inode_unlock();
 
 #ifdef CONFIG_FS_NOTIFY
@@ -274,8 +259,6 @@ errout_with_lock:
     }
 #endif
 
-errout:
-  RELEASE_SEARCH(&newdesc);
   if (subdir != NULL)
     {
       fs_heap_free(subdir);

fs/vfs/fs_unlink.c

@@ -121,14 +121,6 @@ int nx_unlink(FAR const char *pathname)
           goto errout_with_inode;
         }
 
-      /* Verify parent-directory write permission before unlink. */
-
-      ret = inode_checkperm(desc.parent, W_OK);
-      if (ret < 0)
-        {
-          goto errout_with_inode;
-        }
-
       /* Notify the driver that it has been unlinked.  If there are no
        * open references to the driver instance, then the driver should
        * release all resources because it is no longer accessible.

include/nuttx/sched.h

@@ -118,7 +118,9 @@
 #define GROUP_FLAG_DELETED         (1 << 2)                      /* Bit 2: Group has been deleted but not yet freed */
 #define GROUP_FLAG_EXITING         (1 << 3)                      /* Bit 3: Group exit is in progress */
 #define GROUP_FLAG_FD_BACKTRACE    (1 << 4)                      /* Bit 4: Enable FD backtrace for the group */
-                                                                 /* Bits 5-7: Available */
+#define GROUP_FLAG_SECURE_EXEC     (1 << 5)                      /* Bit 5: Secure (setuid/setgid) executable */
+#define GROUP_FLAG_DUMPABLE        (1 << 6)                      /* Bit 6: Process may be traced / coredumped */
+                                                                 /* Bit 7: Available */
 
 /* Values for struct child_status_s ch_flags */
 

include/stdlib.h

@@ -173,6 +173,7 @@ uint32_t  arc4random(void);
 
 FAR char **get_environ_ptr(void);
 FAR char *getenv(FAR const char *name);
+FAR char *secure_getenv(FAR const char *name);
 int       putenv(FAR const char *string);
 int       clearenv(void);
 int       setenv(FAR const char *name, FAR const char *value, int overwrite);

libs/libc/unistd/CMakeLists.txt

@@ -89,7 +89,8 @@ if(NOT CONFIG_SCHED_USER_IDENTITY)
     lib_setreuid.c
     lib_setregid.c
     lib_getresuid.c
-    lib_getresgid.c)
+    lib_getresgid.c
+    lib_issetugid.c)
 endif()
 
 if(NOT CONFIG_DISABLE_ENVIRON)

libs/libc/unistd/Make.defs

@@ -40,6 +40,7 @@ ifneq ($(CONFIG_SCHED_USER_IDENTITY),y)
 CSRCS += lib_setuid.c lib_setgid.c lib_getuid.c lib_getgid.c
 CSRCS += lib_seteuid.c lib_setegid.c lib_geteuid.c lib_getegid.c
 CSRCS += lib_setreuid.c lib_setregid.c lib_getresuid.c lib_getresgid.c
+CSRCS += lib_issetugid.c
 endif
 
 ifneq ($(CONFIG_DISABLE_ENVIRON),y)

libs/libc/unistd/lib_issetugid.c

@@ -0,0 +1,46 @@
+/****************************************************************************
+ * libs/libc/unistd/lib_issetugid.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: issetugid
+ *
+ * Description:
+ *   Stub version always reports that the process is not executing a secure
+ *   set-user-ID or set-group-ID image.  When CONFIG_SCHED_USER_IDENTITY is
+ *   enabled, sched/group/group_issetugid.c provides the real version.
+ *
+ ****************************************************************************/
+
+int issetugid(void)
+{
+  return 0;
+}

sched/environ/CMakeLists.txt

@@ -33,5 +33,6 @@ if(NOT CONFIG_DISABLE_ENVIRON)
             env_putenv.c
             env_setenv.c
             env_unsetenv.c
-            env_foreach.c)
+            env_foreach.c
+            env_secureexec.c)
 endif()

sched/environ/Make.defs

@@ -24,7 +24,7 @@ ifneq ($(CONFIG_DISABLE_ENVIRON),y)
 
 CSRCS += env_getenvironptr.c env_dup.c env_release.c env_findvar.c
 CSRCS += env_removevar.c env_clearenv.c env_getenv.c env_putenv.c
-CSRCS += env_setenv.c env_unsetenv.c env_foreach.c
+CSRCS += env_setenv.c env_unsetenv.c env_foreach.c env_secureexec.c
 
 # Include environ build support
 

sched/environ/env_secureexec.c

@@ -0,0 +1,126 @@
+/****************************************************************************
+ * sched/environ/env_secureexec.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdbool.h>
+#include <string.h>
+#include <stdlib.h>
+#include <assert.h>
+
+#include "sched/sched.h"
+#include "environ/environ.h"
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_USER_IDENTITY
+
+/****************************************************************************
+ * Name: env_is_unsafe
+ *
+ * Description:
+ *   Return true if the "NAME=value" pair must not be inherited by a secure
+ *   (set-user-ID or set-group-ID) executable.
+ *
+ ****************************************************************************/
+
+static bool env_is_unsafe(FAR const char *pair)
+{
+  static FAR const char * const names[] =
+    {
+      "IFS=",
+      "CDPATH=",
+      "ENV=",
+      "BASH_ENV=",
+      "PATH=",
+      "HOSTALIASES=",
+      "LOCPATH=",
+      "GCONV_PATH=",
+      "TZDIR=",
+      "LOCALDOMAIN=",
+      "NLSPATH=",
+      NULL
+    };
+
+  FAR const char * const *name;
+
+  for (name = names; *name != NULL; name++)
+    {
+      if (strncmp(pair, *name, strlen(*name)) == 0)
+        {
+          return true;
+        }
+    }
+
+  return strncmp(pair, "LD_", 3) == 0 ||
+         strncmp(pair, "MALLOC_", 7) == 0;
+}
+
+#endif /* CONFIG_SCHED_USER_IDENTITY */
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: env_sanitize_secure
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_USER_IDENTITY
+void env_sanitize_secure(FAR struct task_group_s *group)
+{
+  ssize_t index;
+
+  DEBUGASSERT(group != NULL);
+
+  nxrmutex_lock(&group->tg_mutex);
+
+  for (index = group->tg_envc - 1; index >= 0; )
+    {
+      if (env_is_unsafe(group->tg_envp[index]))
+        {
+          env_removevar(group, index);
+        }
+      else
+        {
+          index--;
+        }
+    }
+
+  nxrmutex_unlock(&group->tg_mutex);
+}
+#endif
+
+/****************************************************************************
+ * Name: secure_getenv
+ ****************************************************************************/
+
+FAR char *secure_getenv(FAR const char *name)
+{
+  return issetugid() ? NULL : getenv(name);
+}

sched/environ/environ.h

@@ -104,6 +104,19 @@ int env_dup(FAR struct task_group_s *group, FAR char * const *envp);
 
 void env_release(FAR struct task_group_s *group);
 
+#ifdef CONFIG_SCHED_USER_IDENTITY
+/****************************************************************************
+ * Name: env_sanitize_secure
+ *
+ * Description:
+ *   Remove environment variables that must not be inherited by a secure
+ *   (set-user-ID or set-group-ID) executable.
+ *
+ ****************************************************************************/
+
+void env_sanitize_secure(FAR struct task_group_s *group);
+#endif
+
 /****************************************************************************
  * Name: env_findvar
  *

sched/group/CMakeLists.txt

@@ -56,7 +56,8 @@ if(CONFIG_SCHED_USER_IDENTITY)
     group_setreuid.c
     group_setregid.c
     group_getresuid.c
-    group_getresgid.c)
+    group_getresgid.c
+    group_issetugid.c)
 endif()
 
 if(CONFIG_SIG_SIGSTOP_ACTION)

sched/group/Make.defs

@@ -42,6 +42,7 @@ ifeq ($(CONFIG_SCHED_USER_IDENTITY),y)
 CSRCS += group_setuid.c group_setgid.c group_getuid.c group_getgid.c
 CSRCS += group_seteuid.c group_setegid.c group_geteuid.c group_getegid.c
 CSRCS += group_setreuid.c group_setregid.c group_getresuid.c group_getresgid.c
+CSRCS += group_issetugid.c
 endif
 
 ifeq ($(CONFIG_SIG_SIGSTOP_ACTION),y)

sched/group/group_create.c

@@ -171,6 +171,8 @@ int group_allocate(FAR struct tcb_s *tcb, uint8_t ttype)
   group->tg_flags |= GROUP_FLAG_FD_BACKTRACE;
 #endif
 
+  group->tg_flags |= GROUP_FLAG_DUMPABLE;
+
   /* Attach the group to the TCB */
 
   tcb->group = group;

sched/group/group_issetugid.c

@@ -0,0 +1,53 @@
+/****************************************************************************
+ * sched/group/group_issetugid.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+
+#include "sched/sched.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: issetugid
+ *
+ * Description:
+ *   Return non-zero if the process is executing a set-user-ID or
+ *   set-group-ID program image.
+ *
+ ****************************************************************************/
+
+int issetugid(void)
+{
+  FAR struct tcb_s *rtcb          = this_task();
+  FAR struct task_group_s *rgroup = rtcb->group;
+
+  DEBUGASSERT(rgroup != NULL);
+  return (rgroup->tg_flags & GROUP_FLAG_SECURE_EXEC) != 0;
+}

sched/sched/sched.h

@@ -307,6 +307,8 @@ extern volatile spinlock_t g_cpu_tasklistlock;
  * Public Function Prototypes
  ****************************************************************************/
 
+int issetugid(void);
+
 int nxthread_create(FAR const char *name, uint8_t ttype, int priority,
                     FAR void *stack_addr, int stack_size, main_t entry,
                     FAR char * const argv[], FAR char * const envp[]);

sched/task/task_prctl.c

@@ -148,6 +148,46 @@ int prctl(int option, ...)
         goto errout;
 #endif
 
+      case PR_SET_DUMPABLE:
+      case PR_GET_DUMPABLE:
+#ifdef CONFIG_SCHED_USER_IDENTITY
+        {
+          FAR struct tcb_s *tcb = this_task();
+
+          if (tcb == NULL || tcb->group == NULL)
+            {
+              errcode = ESRCH;
+              goto errout;
+            }
+
+          if (option == PR_GET_DUMPABLE)
+            {
+              va_end(ap);
+              return (tcb->group->tg_flags & GROUP_FLAG_DUMPABLE) != 0;
+            }
+
+          if (va_arg(ap, int) != 0)
+            {
+              if (tcb->group->tg_euid != 0)
+                {
+                  errcode = EPERM;
+                  goto errout;
+                }
+
+              tcb->group->tg_flags |= GROUP_FLAG_DUMPABLE;
+            }
+          else
+            {
+              tcb->group->tg_flags &= ~GROUP_FLAG_DUMPABLE;
+            }
+        }
+        break;
+#else
+        serr("ERROR: Option not enabled: %d\n", option);
+        errcode = ENOSYS;
+        goto errout;
+#endif
+
       default:
         serr("ERROR: Unrecognized option: %d\n", option);
         errcode = EINVAL;