# Fix QA Notices in sys-apps/sandbox-2.35 (and maybe also some recently reported sandbox errors) # * QA Notice: Package triggers severe warnings which indicate that it # * may exhibit random runtime failures. # * /tmp/portage/sys-apps/sandbox-2.35/work/sandbox-2.35/src/environ.c:211:19: warning: the comparison will always evaluate as ‘true’ for the address of ‘work_dir’ will never be NULL [-Waddress] # * /tmp/portage/sys-apps/sandbox-2.35/work/sandbox-2.35/libsandbox/canonicalize.c:113:41: warning: passing argument 2 to 'restrict'-qualified parameter aliases with argument 1 [-Wrestrict] # * /tmp/portage/sys-apps/sandbox-2.35/work/sandbox-2.35/libsandbox/libsandbox.c:144:9: warning: passing argument 2 to 'restrict'-qualified parameter aliases with argument 1 [-Wrestrict] # # * Please do not file a Gentoo bug and instead report the above QA # * issues directly to the upstream developers of this software. # * Homepage: https://wiki.gentoo.org/wiki/Project:Sandbox diff --git a/libsandbox/canonicalize.c b/libsandbox/canonicalize.c index f742ed4..68391c7 100644 --- a/libsandbox/canonicalize.c +++ b/libsandbox/canonicalize.c @@ -110,10 +110,17 @@ erealpath(const char *name, char *resolved) if (lstat64(rpath, &st)) break; if (S_ISLNK(st.st_mode)) { - ssize_t cnt = readlink(rpath, rpath, path_max); + /* avoid undefined behaviour resulting from passing rpath + * as source and destination buffer to readlink: + * warning: passing argument 2 to 'restrict'-qualified + * parameter aliases with argument 1 [-Wrestrict] + */ + char buffer[path_max]; + ssize_t cnt = readlink(rpath, buffer, sizeof(buffer)); if (cnt == -1) break; - rpath[cnt] = '\0'; + buffer[cnt] = '\0'; + strcpy(rpath,buffer); if (p) { size_t bytes_left = strlen(p); if (bytes_left >= path_max) diff --git a/libsandbox/libsandbox.c b/libsandbox/libsandbox.c index 847b4e2..55347b9 100644 --- a/libsandbox/libsandbox.c +++ b/libsandbox/libsandbox.c @@ -131,7 +131,7 @@ int resolve_dirfd_path(int dirfd, const char *path, char *resolved_path, save_errno(); - size_t at_len = resolved_path_len - 1 - 1 - (path ? strlen(path) : 0); + /*unused: size_t at_len = resolved_path_len - 1 - 1 - (path ? strlen(path) : 0);*/ if (trace_pid) { sprintf(resolved_path, "/proc/%i/fd/%i", trace_pid, dirfd); } else { @@ -141,7 +141,16 @@ int resolve_dirfd_path(int dirfd, const char *path, char *resolved_path, */ sprintf(resolved_path, "%s/%i", sb_get_fd_dir(), dirfd); } - ssize_t ret = readlink(resolved_path, resolved_path, at_len); + + + /* avoid undefined behaviour resulting from passing resolved_path + * as source and destination buffer to readlink: + * C99 warning: passing argument 2 to 'restrict'-qualified + * parameter aliases with argument 1 [-Wrestrict] + */ + char buffer[resolved_path_len]; + + ssize_t ret = readlink(resolved_path, buffer, sizeof(buffer)); if (ret == -1) { /* see comments at end of check_syscall() */ if (errno_is_too_long()) { @@ -153,11 +162,25 @@ int resolve_dirfd_path(int dirfd, const char *path, char *resolved_path, if (errno == ENOENT) errno = EBADF; return -1; - } - resolved_path[ret] = '/'; - resolved_path[ret + 1] = '\0'; - if (path) - strcat(resolved_path, path); + } else if (ret + 1 >= sizeof(buffer)) { + errno = ENOMEM; + sb_debug_dyn("AT_FD LOOKUP: unsufficient buffer space for resolved_path; max len is %ld; %s\n", sizeof(buffer), strerror(errno)); + return -1; + } + + buffer[ret] = '/'; + buffer[ret + 1] = '\0'; + if (path) { + if ( strlen(buffer) + strlen(path) + 1 < sizeof(buffer) ) { + strcat(buffer, path); + } else { + errno = ENOMEM; + sb_debug_dyn("AT_FD LOOKUP: unsufficient buffer space for resolved_path+path; max len is %ld; %s\n", sizeof(buffer), strerror(errno)); + return -1; + } + } + + strcpy(resolved_path,buffer); restore_errno(); return 0; diff --git a/src/environ.c b/src/environ.c index 542dd64..2b28c0b 100644 --- a/src/environ.c +++ b/src/environ.c @@ -208,7 +208,7 @@ static int setup_cfg_vars(struct sandbox_info_t *sandbox_info) if (-1 == setup_access_var(ENV_SANDBOX_WRITE)) return -1; if ((NULL == getenv(ENV_SANDBOX_WRITE)) && - (NULL != sandbox_info->work_dir)) + strlen(sandbox_info->work_dir)) setenv(ENV_SANDBOX_WRITE, sandbox_info->work_dir, 1); if (-1 == setup_access_var(ENV_SANDBOX_PREDICT)) diff --git a/src/sandbox.c b/src/sandbox.c index ed0c7f6..7951d46 100644 --- a/src/sandbox.c +++ b/src/sandbox.c @@ -34,10 +34,12 @@ const char sbio_fallback_path[] = "/dev/stderr"; static int setup_sandbox(struct sandbox_info_t *sandbox_info, bool interactive) { - if (NULL != getenv(ENV_PORTAGE_TMPDIR)) { - /* Portage handle setting SANDBOX_WRITE itself. */ - sandbox_info->work_dir[0] = '\0'; - } else { + /* avoid using uninitialized fields */ + memset(sandbox_info,0,sizeof(*sandbox_info)); + + + if (NULL == getenv(ENV_PORTAGE_TMPDIR)) { + /* Portage does not handle setting SANDBOX_WRITE itself. */ if (NULL == getcwd(sandbox_info->work_dir, SB_PATH_MAX)) { sb_pwarn("failed to get current directory"); return -1; @@ -249,7 +251,7 @@ int main(int argc, char **argv) dputs("Setting up the required environment variables."); /* If not in portage, cd into it work directory */ - if ('\0' != sandbox_info.work_dir[0]) + if (strlen(sandbox_info.work_dir)) if (chdir(sandbox_info.work_dir)) sb_perr("chdir(%s) failed", sandbox_info.work_dir);