From 6390669178ab83f3796879bd3101f6c4212638d1 Mon Sep 17 00:00:00 2001 From: Yosuke Shimizu Date: Mon, 27 Jul 2026 15:20:15 +0900 Subject: [PATCH] Fix pty-req terminal mode size handling when stdin is not a tty --- src/internal.c | 119 +++++++++++++++-------- tests/unit.c | 237 +++++++++++++++++++++++++++++++++++++++++++++ wolfssh/internal.h | 3 + 3 files changed, 320 insertions(+), 39 deletions(-) diff --git a/src/internal.c b/src/internal.c index b53b4aeb2..01cd07417 100644 --- a/src/internal.c +++ b/src/internal.c @@ -19617,8 +19617,12 @@ int SendChannelRequest(WOLFSSH* ssh, byte* name, word32 nameSz) #if defined(WOLFSSH_TERM) && !defined(NO_FILESYSTEM) +/* out is always a TERMINAL_MODES_MAX_SZ buffer, and one byte is held back + * for the terminator CreateMode() appends */ static void TTYWordSet(word32 flag, int type, byte* out, word32* idx) { + if (*idx + TERMINAL_MODE_SZ + 1 > TERMINAL_MODES_MAX_SZ) + return; out[*idx] = type; *idx += 1; c32toa(flag, out + *idx); *idx += UINT32_SZ; } @@ -19629,6 +19633,8 @@ static void TTYWordSet(word32 flag, int type, byte* out, word32* idx) /* sets terminal mode in buffer and advances idx */ static void TTYSet(word32 isSet, int type, byte* out, word32* idx) { + if (*idx + TERMINAL_MODE_SZ + 1 > TERMINAL_MODES_MAX_SZ) + return; if (isSet) isSet = 1; out[*idx] = type; *idx += 1; c32toa(isSet, out + *idx); *idx += UINT32_SZ; @@ -19638,25 +19644,17 @@ static void TTYWordSet(word32 flag, int type, byte* out, word32* idx) { TTYWordSet((flag & 0xFF), type, out, idx); } -#endif /* !USE_WINDOWS_API && !MICROCHIP_PIC32 && !NO_TERMIOS*/ - - -/* create terminal mode string for pseudo-terminal request - * returns size of buffer */ -static int CreateMode(WOLFSSH* ssh, byte* mode) -{ - word32 idx = 0; - #if !defined(USE_WINDOWS_API) && !defined(MICROCHIP_PIC32) && \ - !defined(NO_TERMIOS) + /* fills mode with the terminal settings of stdin + * returns size of buffer, or WS_FATAL_ERROR when there is no terminal */ + static int CreateModeTermios(byte* mode) { WOLFSSH_TERMIOS term; int baud; + word32 idx = 0; - if (tcgetattr(STDIN_FILENO, &term) != 0) { - printf("Couldn't get the original terminal settings.\n"); - return -1; - } + if (tcgetattr(STDIN_FILENO, &term) != 0) + return WS_FATAL_ERROR; /* get baud rate */ baud = (int)cfgetospeed(&term); @@ -19770,18 +19768,44 @@ static int CreateMode(WOLFSSH* ssh, byte* mode) TTYSet((term.c_cflag & CS8), WOLFSSH_CS8, mode, &idx); TTYSet((term.c_cflag & PARENB), WOLFSSH_PARENB, mode, &idx); TTYSet((term.c_cflag & PARODD), WOLFSSH_PARODD, mode, &idx); + + return (int)idx; } - #else - { - /* No termios. Just set the bitrate to 38400. */ +#endif /* !USE_WINDOWS_API && !MICROCHIP_PIC32 && !NO_TERMIOS*/ + + +/* create terminal mode string for pseudo-terminal request + * mode must be a TERMINAL_MODES_MAX_SZ buffer + * returns size of buffer, or WS_FATAL_ERROR when it would not fit */ +static int CreateMode(WOLFSSH* ssh, byte* mode) +{ + word32 idx = 0; + int termRet = WS_FATAL_ERROR; + int ret = WS_FATAL_ERROR; + + #if !defined(USE_WINDOWS_API) && !defined(MICROCHIP_PIC32) && \ + !defined(NO_TERMIOS) + termRet = CreateModeTermios(mode); + if (termRet < 0) + WLOG(WS_LOG_INFO, "No terminal settings, using default modes"); + #endif + + if (termRet < 0) { + /* No terminal settings. Just set the bitrate to 38400. */ TTYWordSet(38400, WOLFSSH_TTY_OP_ISPEED, mode, &idx); TTYWordSet(38400, WOLFSSH_TTY_OP_OSPEED, mode, &idx); } - #endif /* !USE_WINDOWS_API && !MICROCHIP_PIC32 && !NO_TERMIOS */ + else { + idx = (word32)termRet; + } + + if (idx + 1 <= TERMINAL_MODES_MAX_SZ) { + mode[idx++] = WOLFSSH_TTY_OP_END; + ret = (int)idx; + } WOLFSSH_UNUSED(ssh); - mode[idx++] = WOLFSSH_TTY_OP_END; - return idx; + return ret; } @@ -19856,24 +19880,32 @@ static void GetTerminalInfo(word32* width, word32* height, { #ifdef HAVE_SYS_IOCTL_H struct winsize windowSize = { 0,0,0,0 }; - - ioctl(STDOUT_FILENO, TIOCGWINSZ, &windowSize); - *width = (word32)windowSize.ws_col; - *height = (word32)windowSize.ws_row; - *pixWidth = (word32)windowSize.ws_xpixel; - *pixHeight = (word32)windowSize.ws_ypixel; - *term = getenv("TERM"); #elif defined(_MSC_VER) CONSOLE_SCREEN_BUFFER_INFO cs; +#endif + /* defaults for when there is no terminal to ask, overwritten below by + * whatever the platform can actually read */ + *width = TERMINAL_WIDTH_DEFAULT; + *height = TERMINAL_HEIGHT_DEFAULT; + *pixWidth = 0; + *pixHeight = 0; + *term = NULL; + +#ifdef HAVE_SYS_IOCTL_H + if (ioctl(STDOUT_FILENO, TIOCGWINSZ, &windowSize) == 0 && + windowSize.ws_col != 0 && windowSize.ws_row != 0) { + *width = (word32)windowSize.ws_col; + *height = (word32)windowSize.ws_row; + *pixWidth = (word32)windowSize.ws_xpixel; + *pixHeight = (word32)windowSize.ws_ypixel; + } + *term = getenv("TERM"); +#elif defined(_MSC_VER) if (GetConsoleScreenBufferInfo(GetStdHandle(STD_OUTPUT_HANDLE), &cs) != 0) { *width = cs.srWindow.Right - cs.srWindow.Left + 1; *height = cs.srWindow.Bottom - cs.srWindow.Top + 1; } -#else - /* sane defaults for terminal size if not yet supported */ - *width = 80; - *height = 24; #endif } @@ -19888,22 +19920,31 @@ int SendChannelTerminalRequest(WOLFSSH* ssh) WOLFSSH_CHANNEL* channel; const char cType[] = "pty-req"; const char* term = NULL; - byte mode[4096]; - word32 termSz, typeSz, modeSz; + byte mode[TERMINAL_MODES_MAX_SZ]; + word32 termSz = 0, typeSz = 0, modeSz = 0; word32 w = 0, h = 0, pxW = 0, pxH = 0; + int modeRet; WLOG(WS_LOG_DEBUG, "Entering SendChannelTerminalRequest()"); if (ssh == NULL) ret = WS_BAD_ARGUMENT; - GetTerminalInfo(&w, &h, &pxW, &pxH, &term); - if (term == NULL) { - term = "xterm"; + if (ret == WS_SUCCESS) { + GetTerminalInfo(&w, &h, &pxW, &pxH, &term); + if (term == NULL) { + term = "xterm"; + } + termSz = (word32)WSTRLEN(term); + typeSz = (word32)WSTRLEN(cType); + + /* a negative or oversized result would wrap the packet size */ + modeRet = CreateMode(ssh, mode); + if (modeRet < 0 || (word32)modeRet > (word32)sizeof(mode)) + ret = WS_FATAL_ERROR; + else + modeSz = (word32)modeRet; } - termSz = (word32)WSTRLEN(term); - typeSz = (word32)WSTRLEN(cType); - modeSz = CreateMode(ssh, mode); if (ret == WS_SUCCESS) { channel = ChannelFind(ssh, diff --git a/tests/unit.c b/tests/unit.c index 5b210fd28..a32b2e85f 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -79,6 +79,21 @@ #include #endif +/* SendChannelTerminalRequest() reads the terminal settings of stdin, so the + * no-tty case needs POSIX file descriptors to point stdin at /dev/null. */ +#if defined(WOLFSSH_TEST_INTERNAL) && defined(WOLFSSH_TERM) && \ + !defined(NO_FILESYSTEM) && !defined(NO_WOLFSSH_CLIENT) && \ + !defined(USE_WINDOWS_API) && !defined(MICROCHIP_PIC32) && \ + !defined(NO_TERMIOS) && !defined(WOLFSSL_NUCLEUS) && \ + !defined(WOLFSSH_ZEPHYR) && !defined(_WIN32) + #define TEST_TERM_REQUEST_NO_TTY + /* the test pins TERM and juggles descriptors, so it needs malloc/free + * and the environment calls even when the blocks above are not built */ + #include + #include + #include +#endif + #ifdef WOLFSSH_TEST_INTERNAL @@ -5672,6 +5687,221 @@ static int test_DoChannelRequest(void) return result; } +#ifdef TEST_TERM_REQUEST_NO_TTY + +static word32 CaptureUint32(const byte* buf) +{ + return ((word32)buf[0] << 24) | ((word32)buf[1] << 16) | + ((word32)buf[2] << 8) | (word32)buf[3]; +} + +/* Without a tty the terminal settings and window size can't be read. The + * request falls back to the default modes and an 80x24 window. */ +static int test_SendChannelTerminalRequestNoTty(void) +{ + /* bitrate 38400 is 0x9600, sent in and out, then the terminator */ + static const byte expectedModes[] = { + WOLFSSH_TTY_OP_ISPEED, 0x00, 0x00, 0x96, 0x00, + WOLFSSH_TTY_OP_OSPEED, 0x00, 0x00, 0x96, 0x00, + WOLFSSH_TTY_OP_END + }; + static const char ptyReq[] = "pty-req"; + static const char testTerm[] = "vt100"; + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + WOLFSSH_CHANNEL* ch = NULL; + const char* termEnv; + char* termSave = NULL; + int result = 0; + int ret; + int termPinned = 0; + int stdinCopy = -1; + int stdoutCopy = -1; + int devNull = -1; + int inBack, outBack; + word32 termSaveSz; + word32 idx, typeSz, termSz, modeSz; + word32 width, height, pixWidth, pixHeight; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL); + if (ctx == NULL) + return -1470; + wolfSSH_SetIOSend(ctx, CaptureIoSendChanReq); + + ssh = wolfSSH_new(ctx); + if (ssh == NULL) { result = -1471; goto done; } + + ch = ChannelNew(ssh, ID_CHANTYPE_SESSION, + DEFAULT_WINDOW_SZ, DEFAULT_MAX_PACKET_SZ); + if (ch == NULL) { result = -1472; goto done; } + if (ChannelAppend(ssh, ch) != WS_SUCCESS) { + ChannelDelete(ch, ssh->ctx->heap); + result = -1473; + goto done; + } + ch->peerChannel = ch->channel; + ssh->defaultPeerChannelId = ch->peerChannel; + + s_chanReqCaptureSz = 0; + WMEMSET(s_chanReqCapture, 0, sizeof(s_chanReqCapture)); + + /* TERM is echoed into the packet, so pin it rather than let the host + * environment decide how big the capture has to be */ + termEnv = getenv("TERM"); + if (termEnv != NULL) { + termSaveSz = (word32)WSTRLEN(termEnv) + 1; + termSave = (char*)malloc(termSaveSz); + if (termSave == NULL) { result = -1474; goto done; } + WMEMCPY(termSave, termEnv, termSaveSz); + } + if (setenv("TERM", testTerm, 1) != 0) { result = -1475; goto done; } + termPinned = 1; + + /* stdin carries the terminal settings and stdout the window size, so + * both are pointed at /dev/null to model a client with no terminal */ + stdinCopy = dup(STDIN_FILENO); + stdoutCopy = dup(STDOUT_FILENO); + devNull = open("/dev/null", O_RDWR); + if (stdinCopy < 0 || stdoutCopy < 0 || devNull < 0) { + result = -1476; + goto done; + } + if (dup2(devNull, STDIN_FILENO) < 0) { result = -1477; goto done; } + if (dup2(devNull, STDOUT_FILENO) < 0) { result = -1478; goto done; } + + ret = SendChannelTerminalRequest(ssh); + + /* put the streams back before anything is printed, restoring stdout + * even when stdin fails so later tests are not silenced */ + inBack = dup2(stdinCopy, STDIN_FILENO); + outBack = dup2(stdoutCopy, STDOUT_FILENO); + if (inBack < 0 || outBack < 0) { + result = -1479; + } + close(stdinCopy); + stdinCopy = -1; + close(stdoutCopy); + stdoutCopy = -1; + if (result != 0) + goto done; + + if (ret != WS_SUCCESS) { + printf("SendChannelTerminalRequest: ret=%d, expected=%d\n", + ret, WS_SUCCESS); + result = -1480; + goto done; + } + if (s_chanReqCaptureSz == 0 || + s_chanReqCaptureSz >= (word32)sizeof(s_chanReqCapture)) { + printf("SendChannelTerminalRequest: capture sz=%u\n", + s_chanReqCaptureSz); + result = -1481; + goto done; + } + if (CaptureMsgId(s_chanReqCapture, s_chanReqCaptureSz) + != MSGID_CHANNEL_REQUEST) { + result = -1482; + goto done; + } + + /* walk the payload, keeping idx inside the capture at every step */ + idx = LENGTH_SZ + PAD_LENGTH_SZ + MSG_ID_SZ + UINT32_SZ; + if (idx + UINT32_SZ > s_chanReqCaptureSz) { result = -1483; goto done; } + typeSz = CaptureUint32(s_chanReqCapture + idx); + idx += UINT32_SZ; + if (typeSz != (word32)(sizeof(ptyReq) - 1) || + typeSz > s_chanReqCaptureSz - idx) { + printf("SendChannelTerminalRequest: typeSz=%u\n", typeSz); + result = -1484; + goto done; + } + if (WMEMCMP(s_chanReqCapture + idx, ptyReq, typeSz) != 0) { + result = -1485; + goto done; + } + idx += typeSz; + + if (BOOLEAN_SZ + UINT32_SZ > s_chanReqCaptureSz - idx) { + result = -1486; + goto done; + } + idx += BOOLEAN_SZ; + termSz = CaptureUint32(s_chanReqCapture + idx); + idx += UINT32_SZ; + if (termSz != (word32)(sizeof(testTerm) - 1) || + termSz > s_chanReqCaptureSz - idx) { + printf("SendChannelTerminalRequest: termSz=%u\n", termSz); + result = -1487; + goto done; + } + if (WMEMCMP(s_chanReqCapture + idx, testTerm, termSz) != 0) { + result = -1488; + goto done; + } + idx += termSz; + + /* the window size must never be negotiated as 0x0 */ + if (UINT32_SZ * 4 > s_chanReqCaptureSz - idx) { result = -1489; goto done; } + width = CaptureUint32(s_chanReqCapture + idx); + height = CaptureUint32(s_chanReqCapture + idx + UINT32_SZ); + pixWidth = CaptureUint32(s_chanReqCapture + idx + (UINT32_SZ * 2)); + pixHeight = CaptureUint32(s_chanReqCapture + idx + (UINT32_SZ * 3)); + idx += UINT32_SZ * 4; + if (width != TERMINAL_WIDTH_DEFAULT || height != TERMINAL_HEIGHT_DEFAULT || + pixWidth != 0 || pixHeight != 0) { + printf("SendChannelTerminalRequest: window %ux%u (%ux%u px)\n", + width, height, pixWidth, pixHeight); + result = -1490; + goto done; + } + + /* the modes are the fixed fallback list, not whatever termios left */ + if (UINT32_SZ > s_chanReqCaptureSz - idx) { result = -1491; goto done; } + modeSz = CaptureUint32(s_chanReqCapture + idx); + idx += UINT32_SZ; + if (modeSz != (word32)sizeof(expectedModes) || + modeSz > s_chanReqCaptureSz - idx) { + printf("SendChannelTerminalRequest: modeSz=%u, expected=%u\n", + modeSz, (word32)sizeof(expectedModes)); + result = -1492; + goto done; + } + if (WMEMCMP(s_chanReqCapture + idx, expectedModes, modeSz) != 0) { + result = -1493; + goto done; + } + +done: + if (stdinCopy >= 0) { + if (dup2(stdinCopy, STDIN_FILENO) < 0 && result == 0) + result = -1494; + close(stdinCopy); + } + if (stdoutCopy >= 0) { + if (dup2(stdoutCopy, STDOUT_FILENO) < 0 && result == 0) + result = -1495; + close(stdoutCopy); + } + if (termPinned) { + if (termSave != NULL) { + if (setenv("TERM", termSave, 1) != 0 && result == 0) + result = -1496; + } + else if (unsetenv("TERM") != 0 && result == 0) { + result = -1496; + } + } + if (termSave != NULL) + free(termSave); + if (devNull >= 0) + close(devNull); + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); + return result; +} + +#endif /* TEST_TERM_REQUEST_NO_TTY */ + /* IO send that always wants write, the state a peer forces by not reading. */ static int UnitIoSendWantWrite(WOLFSSH* ssh, void* buf, word32 sz, void* ctx) { @@ -11706,6 +11936,13 @@ int wolfSSH_UnitTest(int argc, char** argv) printf("DoChannelSuccess: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED")); testResult = testResult || unitResult; +#ifdef TEST_TERM_REQUEST_NO_TTY + unitResult = test_SendChannelTerminalRequestNoTty(); + printf("SendChannelTerminalRequestNoTty: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; +#endif + unitResult = test_DoChannelFailure(); printf("DoChannelFailure: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED")); testResult = testResult || unitResult; diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 6577b938c..bf9640e0b 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -537,6 +537,9 @@ enum NameIdType { #define LENGTH_SZ UINT32_SZ #define SSH_PROTO_SZ 8 /* "SSH-2.0-" */ #define TERMINAL_MODE_SZ 5 /* opcode byte + argument uint32 */ +#define TERMINAL_MODES_MAX_SZ 4096 +#define TERMINAL_WIDTH_DEFAULT 80 /* used when there is no terminal */ +#define TERMINAL_HEIGHT_DEFAULT 24 #define AEAD_IMP_IV_SZ 4 #define AEAD_EXP_IV_SZ 8 #define AEAD_NONCE_SZ (AEAD_IMP_IV_SZ+AEAD_EXP_IV_SZ)