From patchwork Sat Jun 25 00:27:34 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Michael X-Patchwork-Id: 13371 Received: (qmail 75599 invoked by alias); 25 Jun 2016 00:27:50 -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 75582 invoked by uid 89); 25 Jun 2016 00:27:49 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=BAYES_00, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=6518, __set_errno, H*r:645, Hx-languages-length:2910 X-HELO: mail-pa0-f66.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:subject:date:message-id:mime-version; bh=VbZ2uS50Pyu+ldHY62bW1JfyZZsXJJCZVrwhKdEugzg=; b=Z4loa3YcvKQfn6eeCnNqslgST4qJ86lxianx80ya7+7KnfEsX/Ybad2s6B+O2rQvD2 Mvf9v4JSCh7VJAaVcw9/u8zuS3pl3T0U2GA3XTqZ0ZhTJs/tL2J86Cwq0EMTOvxHqPax z7HRwULLmkmvEBFPb9z2YpSxPIPqw34PYeVhaAd9YNhPMIZ0aG+94Whbi1xQzt7POM98 IE6ZxEnEdnP0w6/XRg/Od+QEjYvgHBQoH58E77JcW21SyrjLFFl/I+dSvYDJT2KaFksf fDEaVLW0st0gOpWsUxm7zj9M3hOsSyJk7QZi5+onehIZ4st/HsJz2q7jQMG6AVr5Ld5G eN5g== X-Gm-Message-State: ALyK8tJUB5JFQSfapJxpuxGbdF46BGGXR09zcd1yn+gnPpohQaCwaOAPd/WeikorJo+UoQ== X-Received: by 10.66.178.49 with SMTP id cv17mr12258347pac.157.1466814457202; Fri, 24 Jun 2016 17:27:37 -0700 (PDT) From: David Michael To: libc-alpha@sourceware.org Subject: [PATCH] gshadow: Handle the parser's full buffer error code Date: Fri, 24 Jun 2016 17:27:34 -0700 Message-ID: <87oa6qyux5.fsf@gmail.com> MIME-Version: 1.0 * gshadow/fgetsgent_r.c (__fgetsgent_r): Return ERANGE when the parse_line function returns its out-of-space error. --- Hi, The fgetgsent function isn't handling errors from parse_line. That means it can run out of buffer space when adding pointers to group members and exit early without setting all members of the static result struct. The static result's members will remain pointing at buffer locations from the previous line, which have been overwritten with incompatible data, causing segfaults after it is returned normally. This was detected due to systemd segfaulting. See: https://github.com/coreos/bugs/issues/1394 If you don't want to mess with your /etc/gshadow to test it, the following program will also segfault (tested on Fedora and CoreOS). Thanks. David #include #include int main() { struct sgrp *entry; char **member; FILE *input = fdopen (0, "rb"); while (entry = fgetsgent (input)) { printf ("%s", entry->sg_namp); for (member = entry->sg_mem; *member; member++) printf(", %s", *member); printf ("\n"); } fclose (input); return 0; } Feed this through stdin. It should fit in the allocated buffer on x86_64 and succeed if you delete the last character from the second line. grp0:*::root grp1:*::somebody.a1,somebody.a2,somebody.a3,somebody.a4,somebody.a5,somebody.a6,somebody.a7,somebody.a8,somebody.a9,somebody.a10,somebody.a11,somebody.a12,somebody.a13,somebody.a14,somebody.a15,somebody.a16,somebody.a17,somebody.a18,somebody.a19,somebody.a20,somebody.a21,somebody.a22,somebody.a23,somebody.a24,somebody.a25,somebody.a26,somebody.a27,somebody.a28,somebody.a29,somebody.a30,somebody.a31,somebody.a32,somebody.a33,somebody.a34,somebody.a35,somebody.a36,somebody.a37,somebody.a38,somebody.a39,somebody.a40,somebody.a41,somebody.a42,somebody.a43,somebody.a44,somebody.a45,somebody.a46,somebody.a47,a1234 grp2:*::root gshadow/fgetsgent_r.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/gshadow/fgetsgent_r.c b/gshadow/fgetsgent_r.c index b70f6fa..8c13c55 100644 --- a/gshadow/fgetsgent_r.c +++ b/gshadow/fgetsgent_r.c @@ -37,6 +37,7 @@ __fgetsgent_r (FILE *stream, struct sgrp *resbuf, char *buffer, size_t buflen, struct sgrp **result) { char *p; + int rc; _IO_flockfile (stream); do @@ -64,11 +65,18 @@ __fgetsgent_r (FILE *stream, struct sgrp *resbuf, char *buffer, size_t buflen, } while (*p == '\0' || *p == '#' || /* Ignore empty and comment lines. */ /* Parse the line. If it is invalid, loop to get the next line of the file to parse. */ - ! parse_line (buffer, (void *) resbuf, (void *) buffer, buflen, - &errno)); + !(rc = parse_line (buffer, (void *) resbuf, + (void *) buffer, buflen, &errno))); _IO_funlockfile (stream); + if (rc < 0) + { + *result = NULL; + __set_errno (ERANGE); + return errno; + } + *result = resbuf; return 0; }