From 5146c002db4434cad511bbdf3922f3a3c09ef27b Mon Sep 17 00:00:00 2001 From: mahiro21h Date: Wed, 15 Jan 2025 07:32:56 -0800 Subject: [PATCH] [External] [stdlib] Fix `input()` segfaults on EOF (#53925) [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 https://github.com/modularml/mojo/issues/3908 Co-authored-by: mahiro21h Closes modularml/mojo#3919 MODULAR_ORIG_COMMIT_REV_ID: c3457f3377bfcfe0379e31fbd31e72ec53fe7516 Signed-off-by: Joshua James Venter --- stdlib/src/builtin/io.mojo | 13 ++++++++--- stdlib/test/builtin/test_issue_3908.mojo | 28 ++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 stdlib/test/builtin/test_issue_3908.mojo diff --git a/stdlib/src/builtin/io.mojo b/stdlib/src/builtin/io.mojo index 465a4cefcf..b4971adf71 100644 --- a/stdlib/src/builtin/io.mojo +++ b/stdlib/src/builtin/io.mojo @@ -68,7 +68,7 @@ struct _fdopen[mode: StringLiteral = "a"]: """Closes the file handle.""" _ = fclose(self.handle) - fn readline(self) -> String: + fn readline(self) raises -> String: """Reads an entire line from stdin or until EOF. Lines are delimited by a newline character. Returns: @@ -95,7 +95,7 @@ struct _fdopen[mode: StringLiteral = "a"]: """ return self.read_until_delimiter("\n") - fn read_until_delimiter(self, delimiter: StringSlice) -> String: + fn read_until_delimiter(self, delimiter: StringSlice) raises -> String: """Reads an entire line from a stream, up to the `delimiter`. Does not include the delimiter in the result. @@ -140,6 +140,13 @@ struct _fdopen[mode: StringLiteral = "a"]: ord(delimiter), self.handle, ) + # Per man getdelim(3), getdelim will return -1 if an error occurs + # (or the user sends EOF without providing any input). We must + # raise an error in this case because otherwise, String() will crash mojo + # if the user sends EOF with no input. + # TODO: check errno to ensure we haven't encountered EINVAL or ENOMEM instead + if bytes_read == -1: + raise Error("EOF") # Copy the buffer (excluding the delimiter itself) into a Mojo String. var s = String(StringRef(buffer, bytes_read - 1)) # Explicitly free the buffer using free() instead of the Mojo allocator. @@ -283,7 +290,7 @@ fn print[ # ===----------------------------------------------------------------------=== # -fn input(prompt: String = "") -> String: +fn input(prompt: String = "") raises -> String: """Reads a line of input from the user. Reads a line from standard input, converts it to a string, and returns that string. diff --git a/stdlib/test/builtin/test_issue_3908.mojo b/stdlib/test/builtin/test_issue_3908.mojo new file mode 100644 index 0000000000..3e468a428f --- /dev/null +++ b/stdlib/test/builtin/test_issue_3908.mojo @@ -0,0 +1,28 @@ +# ===----------------------------------------------------------------------=== # +# Copyright (c) 2024, Modular Inc. All rights reserved. +# +# Licensed under the Apache License v2.0 with LLVM Exceptions: +# https://llvm.org/LICENSE.txt +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# ===----------------------------------------------------------------------=== # +# RUN: echo -n | %mojo %s + +from builtin.io import _fdopen +from testing import testing + + +fn test_read_until_delimiter_raises_eof() raises: + var stdin = _fdopen["r"](0) + with testing.assert_raises(contains="EOF"): + # Assign to a variable to silence a warning about unused String value + # if an error wasn't raised. + var unused = stdin.read_until_delimiter("\n") + + +fn main() raises: + test_read_until_delimiter_raises_eof()