From 58602a28d2c92b603208f3e01c14c169d28cc7b0 Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Thu, 2 Jul 2026 00:53:24 +0300 Subject: yhttpd: port ychat fixes, get it building + stable in Docker (task 9s0) Ports the ychat revival fixes (unsigned/size_t npos truncation, ofstream == NULL, tool::trim OOB, CGI popen -> execve, www.yChat.org links, g++ version gate, config.pl -I., NCURSES/CLI-disabled build) so yhttpd builds on modern GCC in a Rocky Linux 9 container, plus yhttpd-specific fixes found while verifying it under concurrent load: - listen() backlog was hardcoded to 1; bumped to SOMAXCONN. - sock::_close() closed sockets with unread request bytes still in the kernel receive buffer (read_http() only reads the GET line), so Linux sent an abortive RST instead of a FIN, racing the client's read of the response ("connection reset by peer" even though it was delivered). Fixed with a non-blocking, bounded drain before close() - confirmed via tcpdump: RSTs on every response before, zero after, across 140+ requests / concurrent bursts of 20. - Removed a duplicate _make_server_socket() call in start() (wrap.cpp's init_wrapper() already makes it before start() runs) that leaked a fd and would have double-initialized SSL if OPENSSL is ever enabled; caught by fresh-context review, documented honestly in DOCKER.md. - src/configure's dependency-checker predates 64-bit multilib distros (only checked /usr/lib, never /usr/lib64) and was missing an ncur move-aside entry for the NCURSES-disabled build. Added Dockerfile/.dockerignore/DOCKER.md documenting the build, the fixes, and the one known-but-unfixed landmine (a SIGILL heap corruption in sock::_close that reproduces on newer host GCC/glibc but not in the container - latent, not fixed). Co-Authored-By: Claude Sonnet 5 --- yhttpd/src/tool/tool.cpp | 124 +++++++++++++++++++++++++++++------------------ 1 file changed, 76 insertions(+), 48 deletions(-) (limited to 'yhttpd/src/tool') diff --git a/yhttpd/src/tool/tool.cpp b/yhttpd/src/tool/tool.cpp index dbb1f22..90314fe 100644 --- a/yhttpd/src/tool/tool.cpp +++ b/yhttpd/src/tool/tool.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include "tool.h" @@ -104,7 +105,7 @@ list tool::split_string(string s_string, string s_split) { list list_ret; - unsigned i_pos, i_len = s_split.length(); + size_t i_pos, i_len = s_split.length(); while ( (i_pos = s_string.find(s_split)) != string::npos ) { @@ -120,30 +121,19 @@ tool::split_string(string s_string, string s_split) string tool::trim( string s_str ) { - if( s_str.empty() ) - return s_str; - - char c_cur = s_str[0]; - int i_pos = 0; - - // left trim - while ( c_cur == ' '|| c_cur == '\n' || c_cur == '\r' ) - { - s_str.erase(i_pos,1); - c_cur = s_str[++i_pos]; - } - - // right trim - i_pos = s_str.size(); - c_cur = s_str[s_str.size()]; - - while ( c_cur == ' ' || c_cur == '\n' || c_cur == '\0' || c_cur == '\r' ) - { - s_str.erase(i_pos, 1); - c_cur = s_str[--i_pos]; - } - - return s_str; + // Left trim: find first non-whitespace char. + size_t b = 0; + while ( b < s_str.size() && + ( s_str[b] == ' ' || s_str[b] == '\n' || s_str[b] == '\r' ) ) + ++b; + + // Right trim: find last non-whitespace char. + size_t e = s_str.size(); + while ( e > b && + ( s_str[e-1] == ' ' || s_str[e-1] == '\n' || s_str[e-1] == '\r' ) ) + --e; + + return s_str.substr( b, e - b ); } char* @@ -160,7 +150,7 @@ tool::clean_char( char* c_str ) string tool::replace( string s_string, string s_search, string s_replace ) { - unsigned i_pos[2]; + size_t i_pos[2]; for ( i_pos[0] = s_string.find( s_search ); i_pos[0] != string::npos; @@ -201,37 +191,75 @@ tool::int2char( int i_int ) string tool::shell_command( string s_command, method m_method ) { - FILE *file; - char buf[READBUF]; - char *c_pos; - string s_ret = ""; + // Execute the CGI file directly via fork/execve - NOT through a shell. + // The old popen() ran `/bin/sh -c `, so any shell metacharacter + // in the URL-derived request path was a command-injection / RCE vector + // when httpd.enablecgi=true. s_command must be the full path to an + // executable file (reqp's remove_dots already prevents ".." escaping the + // template dir, and stat() rejects non-files). + (void) m_method; // only METH_RETSTRING is used by the CGI path wrap::system_message(SHELLEX); wrap::system_message(s_command); - if( (file=popen(s_command.c_str(), "r")) == NULL ) + struct stat st; + if ( stat(s_command.c_str(), &st) != 0 || ! S_ISREG(st.st_mode) ) + { + wrap::system_message( SHELLER ); + return ""; + } + + int fd[2]; + if ( pipe(fd) != 0 ) + { + wrap::system_message( SHELLER ); + return ""; + } + + pid_t pid = fork(); + if ( pid < 0 ) { + close(fd[0]); close(fd[1]); wrap::system_message( SHELLER ); + return ""; } - else + + if ( pid == 0 ) + { + // child: stdout -> pipe, close inherited fds, exec the file (no shell). + close(fd[0]); + dup2(fd[1], STDOUT_FILENO); + close(fd[1]); + long l_maxfd = sysconf(_SC_OPEN_MAX); + if ( l_maxfd < 0 ) l_maxfd = 256; + for ( long i = STDERR_FILENO + 1; i < l_maxfd; ++i ) + close((int)i); + char* argv[] = { const_cast(s_command.c_str()), (char*) NULL }; + char* envp[] = { (char*) NULL }; + execve( s_command.c_str(), argv, envp ); + _exit(127); + } + + // parent: read the CGI output. + close(fd[1]); + string s_ret; + char buf[READBUF]; + for (;;) { - while(true) - { - if(fgets(buf, READBUF, file) == NULL) - break; - - switch (m_method) - { - case METH_NCURSES: - wrap::system_message( clean_char(buf) ); - break; - default: - s_ret.append("\n" + string(buf)); - } // switch - } - - pclose(file); + ssize_t n = read(fd[0], buf, sizeof(buf)); + if ( n > 0 ) + s_ret.append( buf, n ); + else if ( n == 0 ) + break; + else if ( errno == EINTR ) + continue; + else + break; } + close(fd[0]); + + int i_status; + waitpid( pid, &i_status, 0 ); return s_ret; } -- cgit v1.2.3