config file line length limit

[Damien Miller]

Hi,

This does look like a bug, so it would be great if you could file it on
bugzilla.

Thanks,
Damien

On Sun, 18 Dec 2016, Don Fong wrote:

> To all,
> 
> i think i've found a minor bug in openssh.  i'm writing to the list
> toget input on whether it's really a bug, or an undocumented limit,
> or maybe it's even documented somewhere (although i didn't see
> it documented in ssh_config(5)).  if there is a consensus that this
> is indeed a bug, i'll file it in bugzilla.  i would also like to
> submit the fix.
> 
> the bug is that ssh barfs if my .ssh/config file contains a very
> long comment line.  basically it tries to parse anything beyond
> the 1022-th char as regular input, not a comment.
> 
> i discovered this behavior by accident.  i have an ansible script
> that spins up new AWS instances.  it creates an ssh config stanza
> for eachnew instance.  at the start of the stanza, the script puts
> a commentline containing amazon's JSON description of the instance.
> dueto recent changes made by amazon, the description is longer than
> it used to be, making the comment longer.  then ssh started failing.
> 
> example (using the attached config file verylong.config):
> 
> $ ssh -F verylong.config whatever
> verylong.config: line 9: Bad configuration option: ABCDEFG
> verylong.config: terminating, 1 bad configuration options
> 
> i decided to take a look at the ssh source code.  i think it is
> pretty clear what is going wrong.
> 
> ----- readconf.c:
> 1703         char line[1024];
> ...
> 1730         while (fgets(line, sizeof(line), f)) {
> 1731                 /* Update line number counter. */
> 1732                 linenum++;
> 1733                 if (process_config_line_depth(options, pw, host,
> original_host,
> 1734                     line, filename, linenum, activep, flags, depth)
> != 0)
> 1735                         bad_options++;
> 1736         }
> 
> if fgets() runs across a very long input line, whatever won't fit in
> the given buffer (sizeof line) is left unread on the input stream,
> to be picked up by later reads.  this is the documented behavior of
> fgets().  how long is too long?  the buffer is sized at 1024 chars.
> one char is needed for the null terminator, another one for the
> newline.  so anything longer than 1024-2 = 1022 bytes is too big.
> 
> unf readconf.c as written just naively assumes that fgets() returns the
> entire line.  it makes no attempt to deal with the case where the line
> didn't entirely fit.  so basically, a long line is treated as multiple
> lines.  this is true whether the line was a comment or something else.
> it's just that the behavior stands out more for a comment line, which
> "should" be completely ignored.  besides, there's not much reason for
> any other type of line to be that long, in the config file.
> 
> i see basically the same problem in the libopenssh version of readconf.c.
> 
> IMHO this is a bug.  some might consider it to be a reasonable limit
> on the line length, but in that case it should be documented in
> ssh_config(5).  and in either case, i think line[] should be declared
> using a symbolic constant for the length.
> 
> or, get rid of the fixed-length buffer, and implement a dynamically
> sized buffer instead.  since i've only begun to look at this code,
> i'm not sure this would be a safe thing to do.  is there any other
> code that implicitly assumes that the line length is less than 1024?
> 
> incidentally, i see a strange discrepancy between the openssh-portable
> version and the libopenssh version.  before the while-fgets loop,
> there is this comment (both versions):
> 
> 1095         /*
> 1096          * Mark that we are now processing the options.  This flag
> is turned
> 1097          * on/off by Host specifications.
> 1098          */
> 1099         active = 1;
> 
> but the "active=1" (line 1099) appears only in the libopenssh version,
> not the openssh-portable version.
> 
> 
> 
> 
> 
> 
> 
>