| Summary: | ssh prints bogus error message if config file has very long lines | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Portable OpenSSH | Reporter: | don fong <dfong> | ||||||||||||
| Component: | ssh | Assignee: | Damien Miller <djm> | ||||||||||||
| Status: | CLOSED FIXED | ||||||||||||||
| Severity: | minor | CC: | djm, dtucker | ||||||||||||
| Priority: | P5 | ||||||||||||||
| Version: | 7.4p1 | ||||||||||||||
| Hardware: | All | ||||||||||||||
| OS: | All | ||||||||||||||
| Bug Depends on: | |||||||||||||||
| Bug Blocks: | 2647 | ||||||||||||||
| Attachments: |
|
||||||||||||||
Created attachment 2919 [details]
proposed patch, untested
Created attachment 2922 [details]
slight improvement to patch
Created attachment 2923 [details]
regression test
regression test for long config lines
Created attachment 2958 [details]
fatal if line is at limit
Here's a simpler patch that makes ssh match sshd's behaviour: fatal if the line completely fills the buffer.
To make sure that this doesn't create problems for users who had configuration files that contained lines this long, this also cranks the line buffer size to match sshd's.
Patch committed. This will be in OpenSSH 7.5 the patch(es) that i submitted have some advantages over this fix. * this fix errors out when the line length is exactly 4095 including newline. in this case, the line is not "too long" to fit in the buffer, so the error message is somewhat misleading. my patch correctly handles the case when the line exactly fits in the buffer. * when the error happens, my patch prints a more helpful error message, telling the user what the maximum line length is. * my patch has a regression test. * my patch also documents (in the man page) the fact that there is a limit on the line length. * my patch uses a symbolic constant for the maximum line length. this is a better practice than a hard-coded constant. it is also needed to tie together the code, the regression test, and the documentation. I appreciate your point, but I don't believe those are compelling enough reasons to justify a significantly more complex solution. it is not significantly more complicated. Close all resolved bugs after release of OpenSSH 7.7. |
Created attachment 2918 [details] an example config file to demonstrate the bug for example, if a config file contains a comment line that is 1023+ chars long, the characters at position 1023 and beyond are treated as a separate line - not ignored as they should be. in this example, longline.config has a comment line that is longer than 1023 chars. $ ssh -F longline.config whatever longline.config: line 5: Bad configuration option: ABCDEFG longline.config: terminating, 1 bad configuration options readconf.c uses a buffer of size 1024. one char is needed for the null terminator, another char is needed for the newline. thus the effective limit is 1022 (excluding newline). very similar code exists in libopenssh. it probably needs the fix too.