util: Fix UB in SetStdinEcho when ENOTTY
What changed, and why it matters
This commit fixes a bug in the code that controls whether your password is shown on screen when you type it into Bitcoin Core. Previously, if the program was not connected to a normal terminal (for example, when run from a script or a pipe), the code could read and write random or invalid terminal settings, which is undefined behavior. The fix checks whether stdin is actually a terminal before trying to change its echo setting, and it now handles errors from the underlying system calls instead of ignoring them. The practical security risk is low: it mainly prevents crashes or strange behavior in non-interactive environments, rather than being an exploitable vulnerability.
No urgent action required. The fix should be included in the next regular release. Users running non-interactive or scripted Bitcoin Core operations are no longer exposed to the undefined behavior. Reviewers may want to verify that StdinTerminal() is implemented consistently across platforms.
Security signals we found
Undefined behavior due to use of uninitialized termios struct when tcgetattr fails
Missing error handling on terminal control system calls
Removal of Valgrind suppression for uninitialized bytes in tcsetattr
Local-only code path triggered during passphrase input
Evidence from the diff
SetStdinEcho in src/compat/stdin.cpp is used to disable/enable terminal echo when reading passphrases. Before this patch, it called tcgetattr/tcsetattr on POSIX and GetConsoleMode/SetConsoleMode on Windows without first verifying that stdin is a terminal. When stdin is not a TTY (e.g., piped input, ENOTTY), tcgetattr fails and leaves the termios struct uninitialized; the subsequent tcsetattr then passes potentially uninitialized or stale data to the kernel, causing undefined behavior. The patch adds an early return via StdinTerminal(), checks return values of the terminal APIs, and removes the corresponding Valgrind suppression for uninitialized bytes in tcsetattr. This is a robustness fix for a local, non-network code path.
Changed components
src/compat/stdin.cppSetStdinEcho functionPassphrase/password prompt echo handlingInspect captured patch +17 / −10
diff --git a/contrib/valgrind.supp b/contrib/valgrind.supp
index ef53f380..c7a890aa 100644
--- a/contrib/valgrind.supp
+++ b/contrib/valgrind.supp
@@ -14,12 +14,6 @@
# Note that suppressions may depend on OS and/or library versions.
# Tested on aarch64 and x86_64 with Ubuntu Noble system libs, using clang-16
# and GCC, without gui.
-{
- Suppress uninitialized bytes warning in compat code
- Memcheck:Param
- ioctl(TCSET{S,SW,SF})
- fun:tcsetattr
-}
{
Suppress leaks on shutdown
Memcheck:Leak
diff --git a/src/compat/stdin.cpp b/src/compat/stdin.cpp
index 20540f2a..0579f64a 100644
--- a/src/compat/stdin.cpp
+++ b/src/compat/stdin.cpp
@@ -18,25 +18,38 @@
// https://stackoverflow.com/questions/1413445/reading-a-password-from-stdcin
void SetStdinEcho(bool enable)
{
+ if (!StdinTerminal()) {
+ return;
+ }
#ifdef WIN32
HANDLE hStdin = GetStdHandle(STD_INPUT_HANDLE);
DWORD mode;
- GetConsoleMode(hStdin, &mode);
+ if (!GetConsoleMode(hStdin, &mode)) {
+ fputs("GetConsoleMode failed\n", stderr);
+ return;
+ }
if (!enable) {
mode &= ~ENABLE_ECHO_INPUT;
} else {
mode |= ENABLE_ECHO_INPUT;
}
- SetConsoleMode(hStdin, mode);
+ if (!SetConsoleMode(hStdin, mode)) {
+ fputs("SetConsoleMode failed\n", stderr);
+ }
#else
struct termios tty;
- tcgetattr(STDIN_FILENO, &tty);
+ if (tcgetattr(STDIN_FILENO, &tty) != 0) {
+ fputs("tcgetattr failed\n", stderr);
+ return;
+ }
if (!enable) {
tty.c_lflag &= ~ECHO;
} else {
tty.c_lflag |= ECHO;
}
- (void)tcsetattr(STDIN_FILENO, TCSANOW, &tty);
+ if (tcsetattr(STDIN_FILENO, TCSANOW, &tty) != 0) {
+ fputs("tcsetattr failed\n", stderr);
+ }
#endif
}
Why this scored 34/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.