| Description: CVE-2013-6393: yaml_parser-{un,}roll-indent: fix int overflow in column argument |
| This expands upon the original indent column overflow patch from |
| comment #12. |
| . |
| The default parser indention is represented as an indention of -1. |
| The original patch only modified the type of the column parameter to |
| the roll/unroll functions, changing it from int to size_t to guard |
| against integer overflow. However, there are code paths that call |
| yaml_parser_unroll_indent with a column of -1 in order to reset the |
| parser back to the initial indention. Since the column is now of |
| type size_t and thus unsigned, passing a column value of -1 caused |
| the column to underflow in this case. |
| . |
| This new patch modifies the roll/unroll functions to handle the -1 |
| indent as a special case. In addition, it adds a new function, |
| yaml_parser_reset_indent. It is nearly an exact copy of |
| yaml_parser_unroll_indent, except it does not take a column |
| parameter. Instead it unrolls to a literal -1 indention, which does |
| not suffer from the underflow. |
| . |
| Code paths that previously called yaml_parser_unroll_indent with a |
| column of -1 are updated to call the new yaml_parser_reset_indent |
| function instead. |
| . |
| With this patch instead of the original: |
| . |
| - `make check` still passes |
| . |
| - The reproducer script completes successfully with exit code 0 |
| . |
| - The issue raised by John Haxby has been corrected and exits with SUCCESS |
| Origin: https://bugzilla.redhat.com/show_bug.cgi?id=1033990 |
| Bug-RedHat: https://bugzilla.redhat.com/show_bug.cgi?id=1033990 |
| Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=737076 |
| Last-Update: 2014-01-29 |
| --- |
| # HG changeset patch |
| # User John Eckersberg <jeckersb@redhat.com> |
| # Date 1390870108 18000 |
| # Mon Jan 27 19:48:28 2014 -0500 |
| # Node ID 7179aa474f31e73834adda26b77bfc25bfe5143d |
| # Parent 3e6507fa0c26d20c09f8f468f2bd04aa2fd1b5b5 |
| yaml_parser-{un,}roll-indent: fix int overflow in column argument |
| |
| diff -r 3e6507fa0c26 -r 7179aa474f31 src/scanner.c |
| --- a/src/scanner.c Mon Dec 24 03:51:32 2012 +0000 |
| +++ b/src/scanner.c Mon Jan 27 19:48:28 2014 -0500 |
| @@ -615,11 +615,14 @@ |
| */ |
| |
| static int |
| -yaml_parser_roll_indent(yaml_parser_t *parser, int column, |
| +yaml_parser_roll_indent(yaml_parser_t *parser, size_t column, |
| int number, yaml_token_type_t type, yaml_mark_t mark); |
| |
| static int |
| -yaml_parser_unroll_indent(yaml_parser_t *parser, int column); |
| +yaml_parser_unroll_indent(yaml_parser_t *parser, size_t column); |
| + |
| +static int |
| +yaml_parser_reset_indent(yaml_parser_t *parser); |
| |
| /* |
| * Token fetchers. |
| @@ -1206,7 +1209,7 @@ |
| */ |
| |
| static int |
| -yaml_parser_roll_indent(yaml_parser_t *parser, int column, |
| +yaml_parser_roll_indent(yaml_parser_t *parser, size_t column, |
| int number, yaml_token_type_t type, yaml_mark_t mark) |
| { |
| yaml_token_t token; |
| @@ -1216,7 +1219,7 @@ |
| if (parser->flow_level) |
| return 1; |
| |
| - if (parser->indent < column) |
| + if (parser->indent == -1 || parser->indent < column) |
| { |
| /* |
| * Push the current indentation level to the stack and set the new |
| @@ -1254,7 +1257,7 @@ |
| |
| |
| static int |
| -yaml_parser_unroll_indent(yaml_parser_t *parser, int column) |
| +yaml_parser_unroll_indent(yaml_parser_t *parser, size_t column) |
| { |
| yaml_token_t token; |
| |
| @@ -1263,6 +1266,15 @@ |
| if (parser->flow_level) |
| return 1; |
| |
| + /* |
| + * column is unsigned and parser->indent is signed, so if |
| + * parser->indent is less than zero the conditional in the while |
| + * loop below is incorrect. Guard against that. |
| + */ |
| + |
| + if (parser->indent < 0) |
| + return 1; |
| + |
| /* Loop through the intendation levels in the stack. */ |
| |
| while (parser->indent > column) |
| @@ -1283,6 +1295,41 @@ |
| } |
| |
| /* |
| + * Pop indentation levels from the indents stack until the current |
| + * level resets to -1. For each intendation level, append the |
| + * BLOCK-END token. |
| + */ |
| + |
| +static int |
| +yaml_parser_reset_indent(yaml_parser_t *parser) |
| +{ |
| + yaml_token_t token; |
| + |
| + /* In the flow context, do nothing. */ |
| + |
| + if (parser->flow_level) |
| + return 1; |
| + |
| + /* Loop through the intendation levels in the stack. */ |
| + |
| + while (parser->indent > -1) |
| + { |
| + /* Create a token and append it to the queue. */ |
| + |
| + TOKEN_INIT(token, YAML_BLOCK_END_TOKEN, parser->mark, parser->mark); |
| + |
| + if (!ENQUEUE(parser, parser->tokens, token)) |
| + return 0; |
| + |
| + /* Pop the indentation level. */ |
| + |
| + parser->indent = POP(parser, parser->indents); |
| + } |
| + |
| + return 1; |
| +} |
| + |
| +/* |
| * Initialize the scanner and produce the STREAM-START token. |
| */ |
| |
| @@ -1338,7 +1385,7 @@ |
| |
| /* Reset the indentation level. */ |
| |
| - if (!yaml_parser_unroll_indent(parser, -1)) |
| + if (!yaml_parser_reset_indent(parser)) |
| return 0; |
| |
| /* Reset simple keys. */ |
| @@ -1369,7 +1416,7 @@ |
| |
| /* Reset the indentation level. */ |
| |
| - if (!yaml_parser_unroll_indent(parser, -1)) |
| + if (!yaml_parser_reset_indent(parser)) |
| return 0; |
| |
| /* Reset simple keys. */ |
| @@ -1407,7 +1454,7 @@ |
| |
| /* Reset the indentation level. */ |
| |
| - if (!yaml_parser_unroll_indent(parser, -1)) |
| + if (!yaml_parser_reset_indent(parser)) |
| return 0; |
| |
| /* Reset simple keys. */ |