Skip to content

Commit

Permalink
Fix #79100: Wrong FTP error messages
Browse files Browse the repository at this point in the history 
First we need to properly clear the `inbuf`, what is an amendment to
commit d2881ad[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] <http://git.php.net/?p=php-src.git;a=commit;h=d2881adcbc9be60de7e7d45a3316b0e11b7eb1e8>.

Closes GH-6718.
  • Loading branch information
@cmb69
cmb69 committed May 3, 2021
1 parent 272df44 commit 42c72ef
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 42 deletions.
1 change: 1 addition & 0 deletions NEWS
View file
Expand Up @@ -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)
Expand Down
49 changes: 27 additions & 22 deletions ext/ftp/ftp.c
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
#ifdef PHP_WIN32
char buf[256];
if (n == 0) {
#ifdef PHP_WIN32
_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;
}

Expand Down Expand Up @@ -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) {
#ifdef PHP_WIN32
char buf[256];
if (n == 0) {
#ifdef PHP_WIN32
_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;
}

Expand Down Expand Up @@ -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) {
#ifdef PHP_WIN32
char buf[256];
if (n == 0) {
#ifdef PHP_WIN32
_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;
}

Expand All @@ -1607,15 +1612,15 @@ data_writeable(ftpbuf_t *ftp, php_socket_t s)

n = php_pollfd_for_ms(s, POLLOUT, 1000);
if (n < 1) {
#ifdef PHP_WIN32
char buf[256];
if (n == 0) {
#ifdef PHP_WIN32
_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;
}

Expand All @@ -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) {
#ifdef PHP_WIN32
char buf[256];
if (n == 0) {
#ifdef PHP_WIN32
_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;
}

Expand Down
80 changes: 60 additions & 20 deletions ext/ftp/php_ftp.c
View file
Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand All @@ -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;
}

Expand All @@ -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;
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand All @@ -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;
}

Expand Down
22 changes: 22 additions & 0 deletions ext/ftp/tests/bug79100.phpt
View file
@@ -0,0 +1,22 @@
--TEST--
Bug #79100 (Wrong FTP error messages)
--SKIPIF--
<?php
require 'skipif.inc';
?>
--FILE--
<?php
$bug79100 = true;
require 'server.inc';

$ftp = ftp_connect("127.0.0.1", $port);
if (!$ftp) die("Couldn't connect to the server");
var_dump(ftp_login($ftp, 'user', 'pass'));
var_dump(ftp_set_option($ftp, FTP_TIMEOUT_SEC, 1));
ftp_systype($ftp);
?>
--EXPECTF--
bool(true)
bool(true)

Warning: ftp_systype(): %rConnection|Operation%r timed out in %s on line %d
2 changes: 2 additions & 0 deletions ext/ftp/tests/server.inc
View file
Expand Up @@ -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 {
Expand Down

0 comments on commit 42c72ef

Please sign in to comment.