From patchwork Wed Jun 11 20:52:18 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 358887 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 547191400EE for ; Thu, 12 Jun 2014 06:52:33 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:subject :content-type; q=dns; s=default; b=ndrvu0sq8+9rU3eBjoze4kCU6ozyj /Zg5eY7NyOLX8hu2Z8DdgweiLXtP/AvAiRdhuvPQdaLCfP0Xq9WlJqv9hdk3G0hH iSMrVvgQG7k25dI02Z3V7t+kKj3I4ls/UvdopnNv5zZJml23RqeNAS2TFDTtBb/c MBfQ+ufPWJCGV0= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:subject :content-type; s=default; bh=4sYNtsdVdTRrhYCzCJWgzWmogok=; b=v2c mewhsGCQLq1vcP+mn1tL2TzGWI+Jbqrf98YmDlx+Wfs0EMJA4Kq6VqxM9n5qdK26 wt/wGAJhQs8GMAwxAvXuNoBPb9wmTchSHoEBZ0FhkbEubzx5VkuIEuqA7GpXi8HW JX9+FO3IS5RgnZQ1LDKQh5isUXwWebWzdYlnFw2I= Received: (qmail 29341 invoked by alias); 11 Jun 2014 20:52:27 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 29331 invoked by uid 89); 11 Jun 2014 20:52:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.8 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Message-ID: <5398C182.4040906@redhat.com> Date: Wed, 11 Jun 2014 22:52:18 +0200 From: Florian Weimer User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 MIME-Version: 1.0 To: GNU C Library Subject: [PATCH] posix_spawn_file_actions_addopen needs to copy the path argument (BZ 17048) POSIX requires that we make a copy, so we allocate a new string and free it in posix_spawn_file_actions_destroy. The reporters (David Reid, Alex Gaynor, and Glyph Lefkowitz) are concerned that not the old behavior could result in security vulnerabilities in applications, and I agree that this cannot be ruled out. 2014-06-11 Florian Weimer * posix/spawn_int.h (struct __spawn_action): Make the path string non-const to support deallocation. * posix/spawn_faction_addopen.c (posix_spawn_file_actions_addopen): Make a copy of the pathname. * posix/spawn_faction_destroy.c (posix_spawn_file_actions_destroy): Adjust comment. Deallocate path in all spawn_do_open actions. * posix/tst-spawn.c (do_test): Exercise the copy operation in posix_spawn_file_actions_addopen. diff --git a/NEWS b/NEWS index 622cdbf..a76ba96 100644 --- a/NEWS +++ b/NEWS @@ -19,7 +19,7 @@ Version 2.20 16791, 16796, 16799, 16800, 16815, 16823, 16824, 16831, 16838, 16849, 16854, 16876, 16877, 16878, 16882, 16885, 16888, 16890, 16912, 16915, 16916, 16917, 16922, 16927, 16928, 16932, 16943, 16958, 16965, 16966, - 16967, 16977, 16978, 16984, 16990, 17009. + 16967, 16977, 16978, 16984, 16990, 17009, 17048. * The minimum Linux kernel version that this version of the GNU C Library can be used with is 2.6.32. diff --git a/posix/spawn_faction_addopen.c b/posix/spawn_faction_addopen.c index 47f6242..40800b8 100644 --- a/posix/spawn_faction_addopen.c +++ b/posix/spawn_faction_addopen.c @@ -35,17 +35,24 @@ posix_spawn_file_actions_addopen (posix_spawn_file_actions_t *file_actions, if (fd < 0 || fd >= maxfd) return EBADF; + char *path_copy = strdup (path); + if (path_copy == NULL) + return ENOMEM; + /* Allocate more memory if needed. */ if (file_actions->__used == file_actions->__allocated && __posix_spawn_file_actions_realloc (file_actions) != 0) - /* This can only mean we ran out of memory. */ - return ENOMEM; + { + /* This can only mean we ran out of memory. */ + free (path_copy); + return ENOMEM; + } /* Add the new value. */ rec = &file_actions->__actions[file_actions->__used]; rec->tag = spawn_do_open; rec->action.open_action.fd = fd; - rec->action.open_action.path = path; + rec->action.open_action.path = path_copy; rec->action.open_action.oflag = oflag; rec->action.open_action.mode = mode; diff --git a/posix/spawn_faction_destroy.c b/posix/spawn_faction_destroy.c index 4d165aa..16d5b69 100644 --- a/posix/spawn_faction_destroy.c +++ b/posix/spawn_faction_destroy.c @@ -18,11 +18,29 @@ #include #include -/* Initialize data structure for file attribute for `spawn' call. */ +#include "spawn_int.h" + +/* Deallocated the file actions. */ int posix_spawn_file_actions_destroy (posix_spawn_file_actions_t *file_actions) { - /* Free the memory allocated. */ + /* Free the paths in the open actions. */ + for (int i = 0; i < file_actions->__used; ++i) + { + struct __spawn_action *sa = file_actions->__actions + i; + switch (sa->tag) + { + case spawn_do_open: + free (sa->action.open_action.path); + break; + case spawn_do_close: + case spawn_do_dup2: + /* No cleanup required. */ + break; + } + } + + /* Free the array of actions. */ free (file_actions->__actions); return 0; } diff --git a/posix/spawn_int.h b/posix/spawn_int.h index 5609e58..861e3b4 100644 --- a/posix/spawn_int.h +++ b/posix/spawn_int.h @@ -22,7 +22,7 @@ struct __spawn_action struct { int fd; - const char *path; + char *path; int oflag; mode_t mode; } open_action; diff --git a/posix/tst-spawn.c b/posix/tst-spawn.c index 84cecf2..6cd874a 100644 --- a/posix/tst-spawn.c +++ b/posix/tst-spawn.c @@ -168,6 +168,7 @@ do_test (int argc, char *argv[]) char fd2name[18]; char fd3name[18]; char fd4name[18]; + char *name3_copy; char *spargv[12]; int i; @@ -222,9 +223,15 @@ do_test (int argc, char *argv[]) if (posix_spawn_file_actions_addclose (&actions, fd1) != 0) error (EXIT_FAILURE, errno, "posix_spawn_file_actions_addclose"); /* We want to open the third file. */ - if (posix_spawn_file_actions_addopen (&actions, fd3, name3, + name3_copy = strdup (name3); + if (name3_copy == NULL) + error (EXIT_FAILURE, errno, "strdup"); + if (posix_spawn_file_actions_addopen (&actions, fd3, name3_copy, O_RDONLY, 0666) != 0) error (EXIT_FAILURE, errno, "posix_spawn_file_actions_addopen"); + /* Overwrite the name to check that a copy has been made. */ + memset (name3_copy, 'X', strlen (name3_copy)); + /* We dup the second descriptor. */ fd4 = MAX (2, MAX (fd1, MAX (fd2, fd3))) + 1; if (posix_spawn_file_actions_adddup2 (&actions, fd2, fd4) != 0) @@ -253,6 +260,7 @@ do_test (int argc, char *argv[]) /* Cleanup. */ if (posix_spawn_file_actions_destroy (&actions) != 0) error (EXIT_FAILURE, errno, "posix_spawn_file_actions_destroy"); + free (name3_copy); /* Wait for the child. */ if (waitpid (pid, &status, 0) != pid) -- 1.9.3