blob: 2a11e3f7ec7dcce0eba8f53002d938ec04f814d6 [file] [log] [blame]
Brad Bishop64c979e2019-11-04 13:55:29 -05001From f752ae5cee163253730ff7cdf293e34a91aa5520 Mon Sep 17 00:00:00 2001
2From: "Todd C. Miller" <Todd.Miller@sudo.ws>
3Date: Thu, 10 Oct 2019 10:04:13 -0600
4Subject: [PATCH] Treat an ID of -1 as invalid since that means "no change".
5 Fixes CVE-2019-14287. Found by Joe Vennix from Apple Information Security.
6
7Upstream-Status: Backport [https://github.com/sudo-project/sudo/commit/f752ae5cee163253730ff7cdf293e34a91aa5520]
8CVE: CVE-2019-14287
9
10Signed-off-by: Changqing Li <changqing.li@windriver.com>
11
12---
13 lib/util/strtoid.c | 100 ++++++++++++++++++++++++++++-------------------------
14 1 files changed, 53 insertions(+), 46 deletions(-)
15
16diff --git a/lib/util/strtoid.c b/lib/util/strtoid.c
17index 2dfce75..6b3916b 100644
18--- a/lib/util/strtoid.c
19+++ b/lib/util/strtoid.c
20@@ -49,6 +49,27 @@
21 #include "sudo_util.h"
22
23 /*
24+ * Make sure that the ID ends with a valid separator char.
25+ */
26+static bool
27+valid_separator(const char *p, const char *ep, const char *sep)
28+{
29+ bool valid = false;
30+ debug_decl(valid_separator, SUDO_DEBUG_UTIL)
31+
32+ if (ep != p) {
33+ /* check for valid separator (including '\0') */
34+ if (sep == NULL)
35+ sep = "";
36+ do {
37+ if (*ep == *sep)
38+ valid = true;
39+ } while (*sep++ != '\0');
40+ }
41+ debug_return_bool(valid);
42+}
43+
44+/*
45 * Parse a uid/gid in string form.
46 * If sep is non-NULL, it contains valid separator characters (e.g. comma, space)
47 * If endp is non-NULL it is set to the next char after the ID.
48@@ -62,36 +83,33 @@ sudo_strtoid_v1(const char *p, const char *sep, char **endp, const char **errstr
49 char *ep;
50 id_t ret = 0;
51 long long llval;
52- bool valid = false;
53 debug_decl(sudo_strtoid, SUDO_DEBUG_UTIL)
54
55 /* skip leading space so we can pick up the sign, if any */
56 while (isspace((unsigned char)*p))
57 p++;
58- if (sep == NULL)
59- sep = "";
60+
61+ /* While id_t may be 64-bit signed, uid_t and gid_t are 32-bit unsigned. */
62 errno = 0;
63 llval = strtoll(p, &ep, 10);
64- if (ep != p) {
65- /* check for valid separator (including '\0') */
66- do {
67- if (*ep == *sep)
68- valid = true;
69- } while (*sep++ != '\0');
70+ if ((errno == ERANGE && llval == LLONG_MAX) || llval > (id_t)UINT_MAX) {
71+ errno = ERANGE;
72+ if (errstr != NULL)
73+ *errstr = N_("value too large");
74+ goto done;
75 }
76- if (!valid) {
77+ if ((errno == ERANGE && llval == LLONG_MIN) || llval < INT_MIN) {
78+ errno = ERANGE;
79 if (errstr != NULL)
80- *errstr = N_("invalid value");
81- errno = EINVAL;
82+ *errstr = N_("value too small");
83 goto done;
84 }
85- if (errno == ERANGE) {
86- if (errstr != NULL) {
87- if (llval == LLONG_MAX)
88- *errstr = N_("value too large");
89- else
90- *errstr = N_("value too small");
91- }
92+
93+ /* Disallow id -1, which means "no change". */
94+ if (!valid_separator(p, ep, sep) || llval == -1 || llval == (id_t)UINT_MAX) {
95+ if (errstr != NULL)
96+ *errstr = N_("invalid value");
97+ errno = EINVAL;
98 goto done;
99 }
100 ret = (id_t)llval;
101@@ -108,30 +126,15 @@ sudo_strtoid_v1(const char *p, const char *sep, char **endp, const char **errstr
102 {
103 char *ep;
104 id_t ret = 0;
105- bool valid = false;
106 debug_decl(sudo_strtoid, SUDO_DEBUG_UTIL)
107
108 /* skip leading space so we can pick up the sign, if any */
109 while (isspace((unsigned char)*p))
110 p++;
111- if (sep == NULL)
112- sep = "";
113+
114 errno = 0;
115 if (*p == '-') {
116 long lval = strtol(p, &ep, 10);
117- if (ep != p) {
118- /* check for valid separator (including '\0') */
119- do {
120- if (*ep == *sep)
121- valid = true;
122- } while (*sep++ != '\0');
123- }
124- if (!valid) {
125- if (errstr != NULL)
126- *errstr = N_("invalid value");
127- errno = EINVAL;
128- goto done;
129- }
130 if ((errno == ERANGE && lval == LONG_MAX) || lval > INT_MAX) {
131 errno = ERANGE;
132 if (errstr != NULL)
133@@ -144,28 +147,31 @@ sudo_strtoid_v1(const char *p, const char *sep, char **endp, const char **errstr
134 *errstr = N_("value too small");
135 goto done;
136 }
137- ret = (id_t)lval;
138- } else {
139- unsigned long ulval = strtoul(p, &ep, 10);
140- if (ep != p) {
141- /* check for valid separator (including '\0') */
142- do {
143- if (*ep == *sep)
144- valid = true;
145- } while (*sep++ != '\0');
146- }
147- if (!valid) {
148+
149+ /* Disallow id -1, which means "no change". */
150+ if (!valid_separator(p, ep, sep) || lval == -1) {
151 if (errstr != NULL)
152 *errstr = N_("invalid value");
153 errno = EINVAL;
154 goto done;
155 }
156+ ret = (id_t)lval;
157+ } else {
158+ unsigned long ulval = strtoul(p, &ep, 10);
159 if ((errno == ERANGE && ulval == ULONG_MAX) || ulval > UINT_MAX) {
160 errno = ERANGE;
161 if (errstr != NULL)
162 *errstr = N_("value too large");
163 goto done;
164 }
165+
166+ /* Disallow id -1, which means "no change". */
167+ if (!valid_separator(p, ep, sep) || ulval == UINT_MAX) {
168+ if (errstr != NULL)
169+ *errstr = N_("invalid value");
170+ errno = EINVAL;
171+ goto done;
172+ }
173 ret = (id_t)ulval;
174 }
175 if (errstr != NULL)
176--
1772.7.4
178