Skip to content

Fixes #729: Ignore libvterm string with length -1 #770

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

strager
Copy link

@strager strager commented Jun 15, 2025

libvterm sometimes gives us a string fragment with length 1073741823 [1]. When this happens, we attempt to read 1073741823 bytes, overrunning the buffer and causing a segfault.

Fix the crash by adding a check for -1. Because the number is not exactly -1 (because VTermStringFragment.len is a 30-bit bit field at least in my copy of libvterm), do some C trickery to get the -1 value for comparison.

[1] This -1 length might be a bug in libvterm. I did not investigate further.

libvterm sometimes gives us a string fragment with length 1073741823 [1]. When
this happens, we attempt to read 1073741823 bytes, overrunning the buffer and
causing a segfault.

Fix the crash by adding a check for -1. Because the number is not exactly -1
(because VTermStringFragment.len is a 30-bit bit field at least in my copy of
libvterm), do some C trickery to get the -1 value for comparison.

[1] This -1 length might be a bug in libvterm. I did not investigate further.
@Sbozzolo
Copy link
Collaborator

Sbozzolo commented Jun 17, 2025

Thank you very much!

What happens when frag.len == invalid_string_fragment_len?

@strager
Copy link
Author

strager commented Jun 20, 2025

Are you asking what exactly causes frag.len to be invalid_string_fragment_len? I don't know, but the test case in #729 triggers it.

Are you asking what the code does when frag.len == invalid_string_fragment_len? It'll behave as if frag.len == 0. But that's not 100% true, because the branch on 1176 won't ever be true. Maybe the code would be clearer if I wrote if (frag.len == invalid_string_fragment_len) frag.len = 0; at the top of the function?

@Sbozzolo
Copy link
Collaborator

Are you asking what the code does when frag.len == invalid_string_fragment_len? It'll behave as if frag.len == 0. But that's not 100% true, because the branch on 1176 won't ever be true. Maybe the code would be clearer if I wrote if (frag.len == invalid_string_fragment_len) frag.len = 0; at the top of the function?

Yes, to me that would be clearer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants