Skip to content
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

[stdlib] Fix input() segfaults on EOF #3919

Closed

Conversation

mahiro21h
Copy link

@mahiro21h mahiro21h commented Dec 29, 2024

pressing ctrl-d with no input when input() is called causes mojo to crash because read_until_delimiter() doesn't check the return value of the C function getdelim(). it assumes getdelim() always succeeds and so, in the case of an error, it blindly creates a StringRef with its length set to the return value - 1 (so the length is -2 in this case). this StringRef is then passed to String() which in turn passes the StringRef to memcpy() with a count of -2 and ultimately crashing mojo.

this pr adds a check in read_until_delimiter() to check if getdelim() failed and raise an error if it does, along with a test to ensure read_until_delimiter() continues to behave as it should.

related issue: #3908
closes: #3908

@mahiro21h mahiro21h requested a review from a team as a code owner December 29, 2024 11:56
@mahiro21h mahiro21h force-pushed the fix-input-segfaults-on-eof branch from 52df8e1 to df55a1f Compare December 29, 2024 12:36
@mahiro21h mahiro21h changed the title Fix input segfaults on eof [stdlib] Fix input segfaults on EOF Dec 29, 2024
@mahiro21h mahiro21h changed the title [stdlib] Fix input segfaults on EOF [stdlib] Fix input() segfaults on EOF Dec 29, 2024
Copy link
Collaborator

@JoeLoser JoeLoser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

@JoeLoser
Copy link
Collaborator

!sync

@modularbot modularbot added the imported-internally Signals that a given pull request has been imported internally. label Jan 14, 2025
@modularbot
Copy link
Collaborator

✅🟣 This contribution has been merged 🟣✅

Your pull request has been merged to the internal upstream Mojo sources. It will be reflected here in the Mojo repository on the nightly branch during the next Mojo nightly release, typically within the next 24-48 hours.

We use Copybara to merge external contributions, click here to learn more.

@modularbot modularbot added merged-internally Indicates that this pull request has been merged internally merged-externally Merged externally in public mojo repo labels Jan 15, 2025
@modularbot
Copy link
Collaborator

Landed in 4ef6859! Thank you for your contribution 🎉

modularbot pushed a commit that referenced this pull request Jan 16, 2025
[External] [stdlib] Fix `input()` segfaults on EOF

pressing `ctrl-d` with no input when `input()` is called causes mojo to
crash because `read_until_delimiter()` doesn't check the return value of
the C function `getdelim()`. it assumes `getdelim()` always succeeds and
so, in the case of an error, it blindly creates a `StringRef` with its
length set to the return value - 1 (so the length is -2 in this case).
this `StringRef` is then passed to `String()` which in turn passes the
`StringRef` to `memcpy()` with a count of -2 and ultimately crashing
mojo.

this pr adds a check in `read_until_delimiter()` to check if
`getdelim()` failed and raise an error if it does, along with a test to
ensure `read_until_delimiter()` continues to behave as it should.

Fixes #3908

Co-authored-by: mahiro21h <[email protected]>
Closes #3919
MODULAR_ORIG_COMMIT_REV_ID: c3457f3377bfcfe0379e31fbd31e72ec53fe7516
@modularbot modularbot closed this Jan 16, 2025
jjvraw pushed a commit to jjvraw/mojo that referenced this pull request Jan 16, 2025
[External] [stdlib] Fix `input()` segfaults on EOF

pressing `ctrl-d` with no input when `input()` is called causes mojo to
crash because `read_until_delimiter()` doesn't check the return value of
the C function `getdelim()`. it assumes `getdelim()` always succeeds and
so, in the case of an error, it blindly creates a `StringRef` with its
length set to the return value - 1 (so the length is -2 in this case).
this `StringRef` is then passed to `String()` which in turn passes the
`StringRef` to `memcpy()` with a count of -2 and ultimately crashing
mojo.

this pr adds a check in `read_until_delimiter()` to check if
`getdelim()` failed and raise an error if it does, along with a test to
ensure `read_until_delimiter()` continues to behave as it should.

Fixes modularml#3908

Co-authored-by: mahiro21h <[email protected]>
Closes modularml#3919
MODULAR_ORIG_COMMIT_REV_ID: c3457f3377bfcfe0379e31fbd31e72ec53fe7516

Signed-off-by: Joshua James Venter <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
imported-internally Signals that a given pull request has been imported internally. merged-externally Merged externally in public mojo repo merged-internally Indicates that this pull request has been merged internally
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants