| Summary: | Out of bounds read when parsing EscapeChar configuration value | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Portable OpenSSH | Reporter: | Jaak Ristioja <jaak+mindrot> | ||||
| Component: | ssh | Assignee: | Damien Miller <djm> | ||||
| Status: | CLOSED FIXED | ||||||
| Severity: | minor | CC: | djm, dtucker | ||||
| Priority: | P5 | ||||||
| Version: | 6.8p1 | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 2360 | ||||||
| Attachments: |
|
||||||
(In reply to Jaak Ristioja from comment #0) > PS: As an unrelated comment I wish to mention that running ssh > through valgrind's memcheck seems to yield lots of results. There are some leaks, but there shouldn't be any memory faults. Please report any that you see - we fix all memory errors that we see. NB. OpenSSL's AES-NI support causes a lot of false positives under valgrind - you might have to disable AES-NI using: export OPENSSL_ia32cap="~0x200000200000000" Created attachment 2628 [details]
reorder EscapeChar tests to avoid 1-byte OOB read
applied - this will be in OpenSSH 6.9, thanks! Set all RESOLVED bugs to CLOSED with release of OpenSSH 7.1 |
An out of bounds memory read occurs during parsing the value for EscapeChar in the following if-statement in readconf.c:1239: if (arg[0] == '^' && arg[2] == 0 && (u_char) arg[1] >= 64 && (u_char) arg[1] < 128) This is erroneous because arg[2] might be one character off the end of the string. I suggest the first two branches be rewritten as follows: if (arg[1] == 0) // was "else if (strlen(arg) == 1)" value = (u_char) arg[0]; else if (arg[0] == '^' && arg[2] == 0 && (u_char) arg[1] >= 64 && (u_char) arg[1] < 128) value = (u_char) arg[1] & 31; This ensures that all single-character values are handled correctly and arg[2] refers to accessible memory. PS: As an unrelated comment I wish to mention that running ssh through valgrind's memcheck seems to yield lots of results.