Fix parser for array out-of-bounds access panic #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 3 commits
November 9, 2016 17:28
We need to check the number of bytes in names as well as the booleans since both can cause us to be oddly aligned by the time we are finished with the booleans.
If an offset in the string section runs off the end of the string table then the previous code would panic with an array out of bounds issue. Restructure the code to check for an initial offset which is too large and use bytes.IndexByte to hunt for the nul and handle the case where one is not found.
|
ping @Nvveen ptal 😃 |
LK4D4
pushed a commit
to moby/moby
that referenced
this pull request
Jan 27, 2017
Although our use of ANSI codes here is rather simple it is generally good practice to use terminfo in order to be portable to different terminal emulators. Vendor github.com/Nvveen/Gotty (actually my fork with a fix, see Nvveen/Gotty#1) and use that to parse the terminfo files. Note that "\e]2K" (clear entire line) is not covered by terminfo. We can achieve the same end by first clearing from begining of line to cursor (el1="\e]1K") and then clearing from cursor to end of line (el="\e]k"). Test suite has been updated and forces (either directly or by setting $TERM to something highly unlikely to exist) the use of the non-terminfo fallbacks which retains the same output behaviour as previously. This is preferable even to relying on a well-known and relatively static terminfo (like vt102) since even that in principal might have different terminfo encodings. In case terminfo is not available at all for $TERM or doesn't expose the specific capabilities which we use then fall back to the previous manual escapes, with the exception that we avoid "\e]2K" as discussed above. Tested with a manual docker pull with rxvt-unicode ($TERM=rxvt-unicode), xterm ($TERM=xterm), mlterm ($TERM=mlterm) and aterm ($TERM=kterm). Signed-off-by: Ian Campbell <[email protected]>
added 5 commits
April 6, 2017 10:56
Signed-off-by: Ian Campbell <[email protected]>
- Checking if the file is unreadable is not needed, just try to open and read and handle the error. - Upon success just return the result rather than breaking from the loop and falling through, which requires more complex error handling. - Upon overall failure return the error directly. There is no need to fmt.Sprintf the static string argument to errors.New() This does not yet correctly handle the case where $TERMINFO contains a value. Signed-off-by: Ian Campbell <[email protected]>
Signed-off-by: Ian Campbell <[email protected]>
https://linux.die.net/man/5/terminfo says: > If the environment variable TERMINFO is set, it is interpreted as the > pathname of a directory containing the compiled description you are working > on. Only that directory is searched. I'd have preferred a more authoritative/OS neutral reference (such as http://pubs.opengroup.org/onlinepubs/7908799/xcurses/terminfo.html) but was unable to find one. Signed-off-by: Ian Campbell <[email protected]>
This was referenced Apr 6, 2017
Using filepath switches from `/` to `\` separators on some platforms which might be unexpected, so defer such a change to another time/discussion. Signed-off-by: Ian Campbell <[email protected]>
added 2 commits
April 6, 2017 12:15
Signed-off-by: Ian Campbell <[email protected]>
fsouza
added a commit
to fsouza/go-dockerclient
that referenced
this pull request
Oct 25, 2017
This revision includes Nvveen/Gotty#1.
tommy351
added a commit
to tommy351/layercake
that referenced
this pull request
Nov 28, 2018
Use a fork version of Gotty to fix the issue Ref: Nvveen/Gotty#1
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I tripped over a panic which was down to my rxvt-unicode terminfo file (on Debian) having an odd length name string but an even number of booleans, meaning the code expected an alignment byte where none was actually present and then later got confused.
This PR fixes that and also updates the string array parsing code to be more robust against bogus offsets in the input. Also fixes a w/space issues.