Segfaults on illegal values in DECSTBM and CBT

Bug #1453611 reported by Mark Lodato
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
libvterm
Fix Committed
Undecided
Unassigned

Bug Description

I have found at least two segfaults in libvterm by running the tests in https://github.com/MarkLodato/vt100-parser/tree/master/test.

Full results with AddressSanitizer stack traces are available at:
https://gist.github.com/MarkLodato/437a6deec280a6e8c68b

1) DECSTBM

Example:
▶ bin/unterm <(printf '\e[10;9r\e[S')
zsh: segmentation fault (core dumped) bin/unterm <(printf '\e[10;9r\e[S')

Here the issue is that libvterm does not validate that the scrolling region's top >= bottom.

2) CBT

Minimal example:
▶ bin/unterm <(printf '\e[Z')
zsh: segmentation fault (core dumped) bin/unterm <(printf '\e[Z')

Here the issue is that libvterm does not validate that the tab stop does not go past the left column.

Both of these bugs should be caught by testing. I highly encourage you to adopt my test cases. You'll have to modify the expected output since I am mirroring xterm and libvterm handles edge cases differently, but it should be a good start. Please let me know if you have any questions.

Mark Lodato (lodatom)
summary: - Two different segfaults regarding scrolling region
+ Segfaults on illegal values in DECSTBM and CBT
Revision history for this message
Paul "LeoNerd" Evans (leonerd) wrote :

I believe this may now be fixed. We've recently been testing it with AFL (http://lcamtuf.coredump.cx/afl/) and that's shaken out quite a few things of this kind.

Please retest on latest version.

Revision history for this message
Mark Lodato (lodatom) wrote :

Yes, these segfaults are now fixed. I still encourage you to add a unit test for this.

Revision history for this message
Mark Lodato (lodatom) wrote :

I also still encourage you to adopt my test suite. There are still lots of differences between libvterm and xterm, and in its current state libvterm causes Neovim to be unusable for me (e.g. when I search through my zsh history, the cursor gets messed up.) By adding my test suite, I think you'll uncover a lot of bugs. If you want, I can spin this off into a separate bug.

Revision history for this message
Paul "LeoNerd" Evans (leonerd) wrote :

Ah - I don't think I ever got around to replying to this before.

Can you further explain "adopt my test suite" - what's involved there?

Changed in libvterm:
status: New → Fix Committed
Revision history for this message
Mark Lodato (lodatom) wrote :

My general suggestion is that libvterm ought to have a comprehensive test suite that lists (input, expected output) for every feature and edge case. This is not really specific to libvterm, so you could make it generic and share it across terminal emulators. When there are intentional differences, they could be documented.

What I have in https://github.com/MarkLodato/vt100-parser/tree/master/test is perhaps a good start. It is a collection of input (`<test>.in`) and expected output (`<test>.in` or `<test>.html`), documenting the behavior of xterm (as of 14 years ago!) for an 80x24 window. The way I implemented it was to open a new xterm window, [rawcat](https://github.com/MarkLodato/vt100-parser/blob/master/rawcat) the input file, then copy and paste the output buffer. If there was scrolling, I copied the full scrollback buffer. (This is what my tool is designed to capture, so it is not an exact match for libvterm.)

For example, [t0003-line_wrap.in](https://github.com/MarkLodato/vt100-parser/blob/master/test/t0003-line_wrap.in) keeps outputting longer and longer lines until it is exceeds 80 columns; [t0003-line_wrap.text](https://github.com/MarkLodato/vt100-parser/blob/master/test/t0003-line_wrap.text) shows the expected output.

For the bug in question, [t0084-CBT.in](https://github.com/MarkLodato/vt100-parser/blob/master/test/t0084-CBT.in) exercises the CBT feature. Here my tool and xterm differ, [t0084-CBT.text](https://github.com/MarkLodato/vt100-parser/blob/master/test/t0084-CBT.text) vs [t0084-CBT.text-xterm](https://github.com/MarkLodato/vt100-parser/blob/master/test/t0084-CBT.text-xterm) respectively.

What I am suggesting is that you could take all of my `.in` and `.text` files and create a small test program that runs each through libvterm, comparing the actual output to the `.text` file. That is exactly what I do in [run_all.py](https://github.com/MarkLodato/vt100-parser/blob/master/test/run_all.py) for my tool.

Happy to help if you need more detail!

Revision history for this message
Mark Lodato (lodatom) wrote :

Also note that libvterm has way more features than my tool, so my test suite would not be complete from libvterm's perspective. But it could be a start.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.