Skip to content

Commit

Permalink
[bugfix] Avoid out of bound access of array in_buffer
Browse files Browse the repository at this point in the history
When pr_comment() calls dump_line() for the first line of a multiline comment, it doesn't include any indentation - it starts with the "/*". This is consistent for both boxed and not boxed comments. Where the logic diverges is in how it treats the rest of the lines of the comment. For box comments indent assumes that it must not change anything, so lines are dumped as they were, including the indentation where it exists. For the rest of comments, it will first remove the indentation to store plain text of the comment and then add it again where indent thinks it's appropriate -- this is part of comment re-indenting process.

For continuations of multi-line comments, the code that handles comments in dump_line() will use pad_output() to create indentation from the beginning of the line (what indent calls the first column) and then write string pointed by s_com afterwards. But if it's a box comment, the string will include original indentation, unless it's the first line of the comment. This is why tab characters from s_com have to be considered when calculating how much padding is needed and the "while (*com_st == '\t') com_st++, target += 8;" does that.

In dump_line(), /target/ is initially set to ps.com_col, so it always assumes that indentation needs to be produced in this function, regardless of which line of a box comment it is. But for the first line of a box comment it is not true, so pr_comment() signals it by setting ps.n_comment_delta, the negative comment delta, to a negative number which is then added to /target/ in dump_line() on all lines except the first one, so that the function produces adequate indentation in this special case.

The bug was in how that negative offset was calculated: pr_comment() used count_spaces() on in_buffer, which pr_comment() expected to contain non-null terminated sequence of characters, originating from whatever originally was on the left side of the comment. Understanding that count_spaces() requires a string, pr_comment() temporarily set buf_ptr[-2] to 0 in hope that it would nul-terminate the right thing in in_buffer and calling count_spaces() would be safe and do the expected thing. This was false whenever buf_ptr would point into save_com, an entirely different char array than in_buffer.

The short-term fix is to recognize whether buf_ptr points into in_buffer or save_com. But I believe that this communication between pr_comment() and dump_line() needs to be removed, just as it was done earlier with regard to star comment continuations in r303598.
  • Loading branch information
pstef committed Nov 13, 2016
1 parent 7898d1a commit ea486a2
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 5 deletions.
5 changes: 3 additions & 2 deletions io.c
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,9 @@ dump_line(void)
char *com_st = s_com;

target += ps.comment_delta;
while (*com_st == '\t')
com_st++, target += 8; /* ? */
while (*com_st == '\t') /* consider original indentation in
* case this is a box comment */
com_st++, target += 8;
while (target <= 0)
if (*com_st == ' ')
target++, com_st++;
Expand Down
13 changes: 10 additions & 3 deletions pr_comment.c
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,16 @@ pr_comment(void)
}
}
if (ps.box_com) {
buf_ptr[-2] = 0;
ps.n_comment_delta = 1 - count_spaces(1, in_buffer);
buf_ptr[-2] = '/';
/*
* Find out how much indentation there was originally, because that
* much will have to be ignored by pad_output() in dump_line(). This
* is a box comment, so nothing changes -- not even indentation.
*
* The comment we're about to read usually comes from in_buffer,
* unless it has been copied into save_com.
*/
char *start = buf_ptr >= save_com && buf_ptr < save_com + sc_size ? bp_save : buf_ptr;
ps.n_comment_delta = 1 - count_spaces_until(1, in_buffer, start - 2);
}
else {
ps.n_comment_delta = 0;
Expand Down

0 comments on commit ea486a2

Please sign in to comment.