xiaoxiang781216 commented on code in PR #19585:
URL: https://github.com/apache/nuttx/pull/19585#discussion_r3695617151
##########
include/unistd.h:
##########
@@ -488,6 +488,7 @@ gid_t getgid(void);
int seteuid(uid_t uid);
uid_t geteuid(void);
+int issetugid(void);
Review Comment:
move to sched/sched/sched.h
##########
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);
Review Comment:
can we remove inode_checkperm after change?
##########
sched/environ/env_secureexec.c:
##########
@@ -0,0 +1,171 @@
+/****************************************************************************
+ * 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>
+
+#ifndef CONFIG_DISABLE_ENVIRON
Review Comment:
remove
##########
sched/environ/env_secureexec.c:
##########
@@ -0,0 +1,171 @@
+/****************************************************************************
+ * 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>
+
+#ifndef CONFIG_DISABLE_ENVIRON
+
+#include <stdbool.h>
+#include <string.h>
+#include <stdlib.h>
+#include <sched.h>
+
+#include <nuttx/sched.h>
+
+#include "sched/sched.h"
+#include "environ/environ.h"
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: env_is_unsafe
+ ****************************************************************************/
+
+static bool env_is_unsafe(FAR const char *name)
+{
+ static const char * const unsafe[] =
+ {
+ "IFS",
+ "CDPATH",
+ "ENV",
+ "BASH_ENV",
+ "PATH",
+ "HOSTALIASES",
+ "LOCPATH",
+ "GCONV_PATH",
+ "TZDIR",
+ "LOCALDOMAIN",
+ "NLSPATH",
+ NULL
+ };
+
+ FAR const char * const *entry;
+
+ for (entry = unsafe; *entry != NULL; entry++)
+ {
+ if (strcmp(name, *entry) == 0)
+ {
+ return true;
+ }
+ }
+
+ if (strncmp(name, "LD_", 3) == 0)
+ {
+ return true;
+ }
+
+ if (strncmp(name, "MALLOC_", 7) == 0)
+ {
+ return true;
+ }
+
+ return false;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: env_sanitize_secure
+ *
+ * Description:
+ * Remove environment variables that must not be inherited by a secure
+ * (set-user-ID or set-group-ID) executable.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_USER_IDENTITY
+void env_sanitize_secure(FAR struct task_group_s *group)
+{
+ FAR char *eq;
+ char name[256];
+ ssize_t index;
+
+ DEBUGASSERT(group != NULL);
+
+ nxrmutex_lock(&group->tg_mutex);
+
+ for (index = group->tg_envc - 1; index >= 0; )
+ {
+ eq = strchr(group->tg_envp[index], '=');
+ if (eq == NULL)
+ {
+ index--;
+ continue;
+ }
+
+ if ((size_t)(eq - group->tg_envp[index]) >= sizeof(name))
+ {
+ index--;
+ continue;
+ }
+
+ memcpy(name, group->tg_envp[index], eq - group->tg_envp[index]);
+ name[eq - group->tg_envp[index]] = '\0';
+
+ if (env_is_unsafe(name))
Review Comment:
let's check the name end with `=` and remove `name` temp variable
##########
binfmt/binfmt_execidentity.c:
##########
@@ -0,0 +1,92 @@
+/****************************************************************************
+ * binfmt/binfmt_execidentity.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 <sys/stat.h>
+
+#include <nuttx/binfmt/binfmt.h>
+#include <nuttx/sched.h>
+
+#include "binfmt.h"
+#include "environ/environ.h"
+
+#ifndef CONFIG_BINFMT_DISABLE
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: binfmt_applyexecidentity
+ *
+ * Description:
+ * Apply set-user-ID and set-group-ID semantics for a newly loaded program
+ * image. When either bit is set in the executable file mode, the
+ * effective and saved set-IDs are updated, the task group is marked as
+ * executing securely, debug state is cleared, and the environment is
+ * sanitized.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_USER_IDENTITY
Review Comment:
remove too
##########
binfmt/binfmt.h:
##########
@@ -231,6 +231,17 @@ void binfmt_freeactions(FAR const
posix_spawn_file_actions_t *copy);
****************************************************************************/
int binfmt_checkexecperm(FAR struct binary_s *bin);
Review Comment:
add blank line
##########
binfmt/binfmt_execidentity.c:
##########
@@ -0,0 +1,92 @@
+/****************************************************************************
+ * binfmt/binfmt_execidentity.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 <sys/stat.h>
+
+#include <nuttx/binfmt/binfmt.h>
+#include <nuttx/sched.h>
+
+#include "binfmt.h"
+#include "environ/environ.h"
+
+#ifndef CONFIG_BINFMT_DISABLE
Review Comment:
remove
##########
sched/group/group_issetugid.c:
##########
@@ -0,0 +1,58 @@
+/****************************************************************************
+ * 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 <unistd.h>
+#include <assert.h>
+
+#include <sched/sched.h>
Review Comment:
change <> to ""
##########
sched/environ/env_secureexec.c:
##########
@@ -0,0 +1,171 @@
+/****************************************************************************
+ * 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>
+
+#ifndef CONFIG_DISABLE_ENVIRON
+
+#include <stdbool.h>
+#include <string.h>
+#include <stdlib.h>
+#include <sched.h>
+
+#include <nuttx/sched.h>
+
+#include "sched/sched.h"
+#include "environ/environ.h"
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: env_is_unsafe
+ ****************************************************************************/
+
+static bool env_is_unsafe(FAR const char *name)
+{
+ static const char * const unsafe[] =
Review Comment:
add FAR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
