CVE-2019-15900: Unchecked Return Value (CWE-252)
value: An issue was discovered in slicer69 doas before 6.2 on certain platforms other than OpenBSD. On platforms without strtonum(3), sscanf was used without checking for error cases. Instead, the uninitialized variable errstr was checked and in some cases returned success even if sscanf failed. The result was that, instead of reporting that the supplied username or group name did not exist, it would execute the command as root.
CWE description: The product does not check the return value from a method or function, which can prevent it from detecting unexpected states and conditions.
CVSS v3 base score: 9.8
Published: 2019-10-18T16:15Z
Repository: https://github.com/slicer69/doas
Language: Other
Files changed: Makefile, doas.c
Commit date: 2019-09-03 11:42:27 -0300
Commit message: Added optimization to Makefile (can be set/overruled using OPT).
Added flag to display all warnings during compiling.
Added status checks when parsing user/group IDs for Linux.
Make sure Linux drops original user's groups when running as another user.

Diff: diff --git a/Makefile b/Makefile
index 7b698e9..890078e 100644
--- a/Makefile
+++ b/Makefile
@@ -3,13 +3,14 @@ YACC?=yacc
 BIN=doas
 PREFIX?=/usr/local
 MANDIR?=$(DESTDIR)$(PREFIX)/man
 SYSCONFDIR?=$(DESTDIR)$(PREFIX)/etc
 OBJECTS=doas.o env.o execvpe.o reallocarray.o y.tab.o
+OPT?=-O2
 # Can set GLOBAL_PATH here to set PATH for target user.
 # TARGETPATH=-DGLOBAL_PATH=\"/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:\"
-CFLAGS+=-DUSE_PAM -DDOAS_CONF=\"${SYSCONFDIR}/doas.conf\" $(TARGETPATH)
+CFLAGS+=-Wall $(OPT) -DUSE_PAM -DDOAS_CONF=\"${SYSCONFDIR}/doas.conf\" $(TARGETPATH)
 LDFLAGS+=-lpam
 UNAME_S := $(shell uname -s)
 ifeq ($(UNAME_S),Linux)
     LDFLAGS+=-lpam_misc
     OBJECTS+=strlcat.o strlcpy.o
diff --git a/doas.c b/doas.c
index 745b1c8..d336720 100644
--- a/doas.c
+++ b/doas.c
@@ -83,23 +83,29 @@ errc(int eval, int code, const char *format)
 
 static int
 parseuid(const char *s, uid_t *uid)
 {
 	struct passwd *pw;
-	const char *errstr;
+	#if !defined(__linux__) && !defined(__NetBSD__)
+	const char *errstr = NULL;
+        #else
+        int status;
+        #endif
 
 	if ((pw = getpwnam(s)) != NULL) {
 		*uid = pw->pw_uid;
 		return 0;
 	}
 	#if !defined(__linux__) && !defined(__NetBSD__)
 	*uid = strtonum(s, 0, UID_MAX, &errstr);
-	#else
-	sscanf(s, "%d", uid);
-	#endif
 	if (errstr)
 		return -1;
+	#else
+	status = sscanf(s, "%d", uid);
+        if (status != 1)
+           return -1;
+	#endif
 	return 0;
 }
 
 static int
 uidcheck(const char *s, uid_t desired)
@@ -115,23 +121,29 @@ uidcheck(const char *s, uid_t desired)
 
 static int
 parsegid(const char *s, gid_t *gid)
 {
 	struct group *gr;
-	const char *errstr;
+	#if !defined(__linux__) && !defined(__NetBSD__)
+	const char *errstr = NULL;
+        #else
+        int status;
+        #endif
 
 	if ((gr = getgrnam(s)) != NULL) {
 		*gid = gr->gr_gid;
 		return 0;
 	}
 	#if !defined(__linux__) && !defined(__NetBSD__)
 	*gid = strtonum(s, 0, GID_MAX, &errstr);
-	#else
-	sscanf(s, "%d", gid);
-	#endif
 	if (errstr)
 		return -1;
+	#else
+	status = sscanf(s, "%d", gid);
+        if (status != 1)
+            return -1;
+	#endif
 	return 0;
 }
 
 static int
 match(uid_t uid, gid_t *groups, int ngroups, uid_t target, const char *cmd,

Vulnerable code: CFLAGS+=-DUSE_PAM -DDOAS_CONF=\"${SYSCONFDIR}/doas.conf\" $(TARGETPATH)
	const char *errstr;
	#else
	sscanf(s, "%d", uid);
	#endif
	const char *errstr;
	#else
	sscanf(s, "%d", gid);
	#endif
Fixed code: OPT?=-O2
CFLAGS+=-Wall $(OPT) -DUSE_PAM -DDOAS_CONF=\"${SYSCONFDIR}/doas.conf\" $(TARGETPATH)
	#if !defined(__linux__) && !defined(__NetBSD__)
	const char *errstr = NULL;
        #else
        int status;
        #endif
	#else
	status = sscanf(s, "%d", uid);
        if (status != 1)
           return -1;
	#endif
	#if !defined(__linux__) && !defined(__NetBSD__)
	const char *errstr = NULL;
        #else
        int status;
        #endif
	#else
	status = sscanf(s, "%d", gid);
        if (status != 1)
            return -1;
	#endif