color: add overflow checks for parsing colors

Our color parsing is designed to never exceed COLOR_MAXLEN bytes. But the relationship between that hand-computed number and the parsing code is not at all obvious, and we merely hope that it has been computed correctly for all cases. Let's mark the expected "end" pointer for the destination buffer and make sure that we do not exceed it. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Sep 24, 2015 at 17:08 UTC cbc8feeaf97db39ff7220c4d1b03e0c3fdd2401c
1 file changed +26 -15
color.c
+26 -15
@@ -150,22 +150,24 @@ int color_parse(const char *value, char *dst)
150 * already have the ANSI escape code in it. "out" should have enough
151 * space in it to fit any color.
152 */
153 -static char *color_output(char *out, const struct color *c, char type)
153 +static char *color_output(char *out, int len, const struct color *c, char type)
154 {
155 switch (c->type) {
156 case COLOR_UNSPECIFIED:
157 case COLOR_NORMAL:
158 break;
159 case COLOR_ANSI:
160 + if (len < 2)
161 + die("BUG: color parsing ran out of space");
162 *out++ = type;
163 *out++ = '0' + c->value;
164 break;
165 case COLOR_256:
164 - out += sprintf(out, "%c8;5;%d", type, c->value);
166 + out += xsnprintf(out, len, "%c8;5;%d", type, c->value);
167 break;
168 case COLOR_RGB:
167 - out += sprintf(out, "%c8;2;%d;%d;%d", type,
168 - c->red, c->green, c->blue);
169 + out += xsnprintf(out, len, "%c8;2;%d;%d;%d", type,
170 + c->red, c->green, c->blue);
171 break;
172 }
173 return out;
@@ -180,12 +182,13 @@ int color_parse_mem(const char *value, int value_len, char *dst)
182 {
183 const char *ptr = value;
184 int len = value_len;
185 + char *end = dst + COLOR_MAXLEN;
186 unsigned int attr = 0;
187 struct color fg = { COLOR_UNSPECIFIED };
188 struct color bg = { COLOR_UNSPECIFIED };
189
190 if (!strncasecmp(value, "reset", len)) {
188 - strcpy(dst, GIT_COLOR_RESET);
191 + xsnprintf(dst, end - dst, GIT_COLOR_RESET);
192 return 0;
193 }
194
@@ -224,12 +227,19 @@ int color_parse_mem(const char *value, int value_len, char *dst)
227 goto bad;
228 }
229
230 +#undef OUT
231 +#define OUT(x) do { \
232 + if (dst == end) \
233 + die("BUG: color parsing ran out of space"); \
234 + *dst++ = (x); \
235 +} while(0)
236 +
237 if (attr || !color_empty(&fg) || !color_empty(&bg)) {
238 int sep = 0;
239 int i;
240
231 - *dst++ = '\033';
232 - *dst++ = '[';
241 + OUT('\033');
242 + OUT('[');
243
244 for (i = 0; attr; i++) {
245 unsigned bit = (1 << i);
@@ -237,27 +247,28 @@ int color_parse_mem(const char *value, int value_len, char *dst)
247 continue;
248 attr &= ~bit;
249 if (sep++)
240 - *dst++ = ';';
241 - dst += sprintf(dst, "%d", i);
250 + OUT(';');
251 + dst += xsnprintf(dst, end - dst, "%d", i);
252 }
253 if (!color_empty(&fg)) {
254 if (sep++)
245 - *dst++ = ';';
255 + OUT(';');
256 /* foreground colors are all in the 3x range */
247 - dst = color_output(dst, &fg, '3');
257 + dst = color_output(dst, end - dst, &fg, '3');
258 }
259 if (!color_empty(&bg)) {
260 if (sep++)
251 - *dst++ = ';';
261 + OUT(';');
262 /* background colors are all in the 4x range */
253 - dst = color_output(dst, &bg, '4');
263 + dst = color_output(dst, end - dst, &bg, '4');
264 }
255 - *dst++ = 'm';
265 + OUT('m');
266 }
257 - *dst = 0;
267 + OUT(0);
268 return 0;
269 bad:
270 return error(_("invalid color value: %.*s"), value_len, value);
271 +#undef OUT
272 }
273
274 int git_config_colorbool(const char *var, const char *value)