Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date: Sat, 28 Sep 2013 23:08:46 -0700
From: Michael Forney <mforney@...rney.org>
To: musl@...ts.openwall.com
Subject: [PATCH] getgr_r: Reserve space for gr_mem's NULL terminator in buffer

Currently, the NULL terminator overlaps with the beginning of the line, causing
gr_name to always be the empty string.
---
As an aside, I don't understand why 32 is added to the size check. It looks
like the length is rounded down to a multiple of 16, so at most 15 extra bytes
will be needed. But even so, wouldn't it be better to check for exactly the
amount of space that will be used? Or is it not worth the additional temporary
variable?

 src/passwd/getgr_r.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/passwd/getgr_r.c b/src/passwd/getgr_r.c
index 234c901..3fe2e2b 100644
--- a/src/passwd/getgr_r.c
+++ b/src/passwd/getgr_r.c
@@ -26,14 +26,14 @@ static int getgr_r(const char *name, gid_t gid, struct group *gr, char *buf, siz
 	while (__getgrent_a(f, gr, &line, &len, &mem, &nmem)) {
 		if (name && !strcmp(name, gr->gr_name)
 		|| !name && gr->gr_gid == gid) {
-			if (size < len + nmem*sizeof(char *) + 32) {
+			if (size < len + (nmem+1)*sizeof(char *) + 32) {
 				rv = ERANGE;
 				break;
 			}
 			*res = gr;
 			buf += (16-(uintptr_t)buf)%16;
 			gr->gr_mem = (void *)buf;
-			buf += nmem*sizeof(char *);
+			buf += (nmem+1)*sizeof(char *);
 			memcpy(buf, line, len);
 			FIX(name);
 			FIX(passwd);
-- 
1.8.4

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.