From 42c72ef463603722e81a6b86443bdb753318b700 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Thu, 22 Apr 2021 18:31:47 +0200 Subject: [PATCH] Fix #79100: Wrong FTP error messages First we need to properly clear the `inbuf`, what is an amendment to commit d2881adcbc[1]. Then we need to report `php_pollfd_for_ms()` failures right away; just setting `errno` does not really help, since at least in some cases it would have been overwritten before we actually could check it. We use `php_socket_strerror()` to get a proper error message, and define `ETIMEDOUT` to the proper value on Windows; otherwise we catch the definition in errno.h, which is not compatible with WinSock. The proper solution for this issue would likely be to include something like ext/sockets/windows_common.h. Finally, we ensure that we only report warnings using `inbuf`, if it is not empty. [1] . Closes GH-6718. --- NEWS | 1 + ext/ftp/ftp.c | 49 +++++++++++++---------- ext/ftp/php_ftp.c | 80 +++++++++++++++++++++++++++---------- ext/ftp/tests/bug79100.phpt | 22 ++++++++++ ext/ftp/tests/server.inc | 2 + 5 files changed, 112 insertions(+), 42 deletions(-) create mode 100644 ext/ftp/tests/bug79100.phpt diff --git a/NEWS b/NEWS index f5284d0d495..3ceb9cc0c6e 100644 --- a/NEWS +++ b/NEWS @@ -13,6 +13,7 @@ PHP NEWS - FTP: . Fixed bug #80901 (Info leak in ftp extension). (cmb) + . Fixed bug #79100 (Wrong FTP error messages). (cmb) - ODBC: . Fixed bug #80460 (ODBC doesn't account for SQL_NO_TOTAL indicator). (cmb) diff --git a/ext/ftp/ftp.c b/ext/ftp/ftp.c index 9b04edcd099..a11aa323c1c 100644 --- a/ext/ftp/ftp.c +++ b/ext/ftp/ftp.c @@ -63,6 +63,11 @@ #include "ftp.h" #include "ext/standard/fsock.h" +#ifdef PHP_WIN32 +# undef ETIMEDOUT +# define ETIMEDOUT WSAETIMEDOUT +#endif + /* sends an ftp command, returns true on success, false on error. * it sends the string "cmd args\r\n" if args is non-null, or * "cmd\r\n" if args is null @@ -1295,7 +1300,8 @@ ftp_putcmd(ftpbuf_t *ftp, const char *cmd, const size_t cmd_len, const char *arg data = ftp->outbuf; - /* Clear the extra-lines buffer */ + /* Clear the inbuf and extra-lines buffer */ + ftp->inbuf[0] = '\0'; ftp->extra = NULL; if (my_send(ftp, ftp->fd, data, size) != size) { @@ -1468,15 +1474,15 @@ my_send(ftpbuf_t *ftp, php_socket_t s, void *buf, size_t len) n = php_pollfd_for_ms(s, POLLOUT, ftp->timeout_sec * 1000); if (n < 1) { + char buf[256]; + if (n == 0) { #ifdef PHP_WIN32 - if (n == 0) { _set_errno(ETIMEDOUT); - } #else - if (n == 0) { errno = ETIMEDOUT; - } #endif + } + php_error_docref(NULL, E_WARNING, "%s", php_socket_strerror(errno, buf, sizeof buf)); return -1; } @@ -1505,18 +1511,17 @@ my_recv(ftpbuf_t *ftp, php_socket_t s, void *buf, size_t len) SSL *handle = NULL; php_socket_t fd; #endif - n = php_pollfd_for_ms(s, PHP_POLLREADABLE, ftp->timeout_sec * 1000); if (n < 1) { + char buf[256]; + if (n == 0) { #ifdef PHP_WIN32 - if (n == 0) { _set_errno(ETIMEDOUT); - } #else - if (n == 0) { errno = ETIMEDOUT; - } #endif + } + php_error_docref(NULL, E_WARNING, "%s", php_socket_strerror(errno, buf, sizeof buf)); return -1; } @@ -1583,15 +1588,15 @@ data_available(ftpbuf_t *ftp, php_socket_t s) n = php_pollfd_for_ms(s, PHP_POLLREADABLE, 1000); if (n < 1) { + char buf[256]; + if (n == 0) { #ifdef PHP_WIN32 - if (n == 0) { _set_errno(ETIMEDOUT); - } #else - if (n == 0) { errno = ETIMEDOUT; - } #endif + } + php_error_docref(NULL, E_WARNING, "%s", php_socket_strerror(errno, buf, sizeof buf)); return 0; } @@ -1607,15 +1612,15 @@ data_writeable(ftpbuf_t *ftp, php_socket_t s) n = php_pollfd_for_ms(s, POLLOUT, 1000); if (n < 1) { + char buf[256]; + if (n == 0) { #ifdef PHP_WIN32 - if (n == 0) { _set_errno(ETIMEDOUT); - } #else - if (n == 0) { errno = ETIMEDOUT; - } #endif + } + php_error_docref(NULL, E_WARNING, "%s", php_socket_strerror(errno, buf, sizeof buf)); return 0; } @@ -1632,15 +1637,15 @@ my_accept(ftpbuf_t *ftp, php_socket_t s, struct sockaddr *addr, socklen_t *addrl n = php_pollfd_for_ms(s, PHP_POLLREADABLE, ftp->timeout_sec * 1000); if (n < 1) { + char buf[256]; + if (n == 0) { #ifdef PHP_WIN32 - if (n == 0) { _set_errno(ETIMEDOUT); - } #else - if (n == 0) { errno = ETIMEDOUT; - } #endif + } + php_error_docref(NULL, E_WARNING, "%s", php_socket_strerror(errno, buf, sizeof buf)); return -1; } diff --git a/ext/ftp/php_ftp.c b/ext/ftp/php_ftp.c index e3b425ef0fd..3818c849d7b 100644 --- a/ext/ftp/php_ftp.c +++ b/ext/ftp/php_ftp.c @@ -455,7 +455,9 @@ PHP_FUNCTION(ftp_login) /* log in */ if (!ftp_login(ftp, user, user_len, pass, pass_len)) { - php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + if (*ftp->inbuf) { + php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + } RETURN_FALSE; } @@ -480,7 +482,9 @@ PHP_FUNCTION(ftp_pwd) } if (!(pwd = ftp_pwd(ftp))) { - php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + if (*ftp->inbuf) { + php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + } RETURN_FALSE; } @@ -504,7 +508,9 @@ PHP_FUNCTION(ftp_cdup) } if (!ftp_cdup(ftp)) { - php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + if (*ftp->inbuf) { + php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + } RETURN_FALSE; } @@ -531,7 +537,9 @@ PHP_FUNCTION(ftp_chdir) /* change directories */ if (!ftp_chdir(ftp, dir, dir_len)) { - php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + if (*ftp->inbuf) { + php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + } RETURN_FALSE; } @@ -558,7 +566,9 @@ PHP_FUNCTION(ftp_exec) /* execute serverside command */ if (!ftp_exec(ftp, cmd, cmd_len)) { - php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + if (*ftp->inbuf) { + php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + } RETURN_FALSE; } @@ -608,7 +618,9 @@ PHP_FUNCTION(ftp_mkdir) /* create directory */ if (NULL == (tmp = ftp_mkdir(ftp, dir, dir_len))) { - php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + if (*ftp->inbuf) { + php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + } RETURN_FALSE; } @@ -635,7 +647,9 @@ PHP_FUNCTION(ftp_rmdir) /* remove directorie */ if (!ftp_rmdir(ftp, dir, dir_len)) { - php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + if (*ftp->inbuf) { + php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + } RETURN_FALSE; } @@ -662,7 +676,9 @@ PHP_FUNCTION(ftp_chmod) } if (!ftp_chmod(ftp, mode, filename, filename_len)) { - php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + if (*ftp->inbuf) { + php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + } RETURN_FALSE; } @@ -816,7 +832,9 @@ PHP_FUNCTION(ftp_systype) } if (NULL == (syst = ftp_syst(ftp))) { - php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + if (*ftp->inbuf) { + php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + } RETURN_FALSE; } @@ -862,7 +880,9 @@ PHP_FUNCTION(ftp_fget) } if (!ftp_get(ftp, stream, file, file_len, xtype, resumepos)) { - php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + if (*ftp->inbuf) { + php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + } RETURN_FALSE; } @@ -912,7 +932,9 @@ PHP_FUNCTION(ftp_nb_fget) ftp->closestream = 0; /* do not close */ if ((ret = ftp_nb_get(ftp, stream, file, file_len, xtype, resumepos)) == PHP_FTP_FAILED) { - php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + if (*ftp->inbuf) { + php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + } RETURN_LONG(ret); } @@ -1000,7 +1022,9 @@ PHP_FUNCTION(ftp_get) if (!ftp_get(ftp, outstream, remote, remote_len, xtype, resumepos)) { php_stream_close(outstream); VCWD_UNLINK(local); - php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + if (*ftp->inbuf) { + php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + } RETURN_FALSE; } @@ -1069,7 +1093,9 @@ PHP_FUNCTION(ftp_nb_get) php_stream_close(outstream); ftp->stream = NULL; VCWD_UNLINK(local); - php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + if (*ftp->inbuf) { + php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + } RETURN_LONG(PHP_FTP_FAILED); } @@ -1163,7 +1189,9 @@ PHP_FUNCTION(ftp_fput) } if (!ftp_put(ftp, remote, remote_len, stream, xtype, startpos)) { - php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + if (*ftp->inbuf) { + php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + } RETURN_FALSE; } @@ -1217,7 +1245,9 @@ PHP_FUNCTION(ftp_nb_fput) ftp->closestream = 0; /* do not close */ if (((ret = ftp_nb_put(ftp, remote, remote_len, stream, xtype, startpos)) == PHP_FTP_FAILED)) { - php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + if (*ftp->inbuf) { + php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + } RETURN_LONG(ret); } @@ -1271,7 +1301,9 @@ PHP_FUNCTION(ftp_put) if (!ftp_put(ftp, remote, remote_len, instream, xtype, startpos)) { php_stream_close(instream); - php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + if (*ftp->inbuf) { + php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + } RETURN_FALSE; } php_stream_close(instream); @@ -1307,7 +1339,9 @@ PHP_FUNCTION(ftp_append) if (!ftp_append(ftp, remote, remote_len, instream, xtype)) { php_stream_close(instream); - php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + if (*ftp->inbuf) { + php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + } RETURN_FALSE; } php_stream_close(instream); @@ -1441,7 +1475,9 @@ PHP_FUNCTION(ftp_rename) /* rename the file */ if (!ftp_rename(ftp, src, src_len, dest, dest_len)) { - php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + if (*ftp->inbuf) { + php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + } RETURN_FALSE; } @@ -1468,7 +1504,9 @@ PHP_FUNCTION(ftp_delete) /* delete the file */ if (!ftp_delete(ftp, file, file_len)) { - php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + if (*ftp->inbuf) { + php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + } RETURN_FALSE; } @@ -1495,7 +1533,9 @@ PHP_FUNCTION(ftp_site) /* send the site command */ if (!ftp_site(ftp, cmd, cmd_len)) { - php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + if (*ftp->inbuf) { + php_error_docref(NULL, E_WARNING, "%s", ftp->inbuf); + } RETURN_FALSE; } diff --git a/ext/ftp/tests/bug79100.phpt b/ext/ftp/tests/bug79100.phpt new file mode 100644 index 00000000000..21876bb3db8 --- /dev/null +++ b/ext/ftp/tests/bug79100.phpt @@ -0,0 +1,22 @@ +--TEST-- +Bug #79100 (Wrong FTP error messages) +--SKIPIF-- + +--FILE-- + +--EXPECTF-- +bool(true) +bool(true) + +Warning: ftp_systype(): %rConnection|Operation%r timed out in %s on line %d \ No newline at end of file diff --git a/ext/ftp/tests/server.inc b/ext/ftp/tests/server.inc index 71f0881dea6..b77bed904f2 100644 --- a/ext/ftp/tests/server.inc +++ b/ext/ftp/tests/server.inc @@ -198,6 +198,8 @@ if ($pid) { } elseif ($buf === "SYST\r\n") { if (isset($bug27809)) { fputs($s, "215 OS/400 is the remote operating system. The TCP/IP version is \"V5R2M0\"\r\n"); + } elseif (isset($bug79100)) { + // do nothing so test hits timeout } elseif (isset($bug80901)) { fputs($s, "\r\n" . str_repeat("*", 4096) . "\r\n"); } else {