From: Martin Kraemer Date: Wed, 30 Jan 2002 14:54:18 +0000 (+0000) Subject: For the ftp commands CWD and RETR, do escaping for the path elements, X-Git-Tag: 2.0.31~18 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e13539178091ec16d01c8fba7fd7e37a2cada510;p=apache For the ftp commands CWD and RETR, do escaping for the path elements, because FTP servers do globbing, which we do not want for filenames from the directory listing. PR: Obtained from: Submitted by: Reviewed by: git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@93105 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/modules/proxy/proxy_ftp.c b/modules/proxy/proxy_ftp.c index 692f0e797a..89a628a678 100644 --- a/modules/proxy/proxy_ftp.c +++ b/modules/proxy/proxy_ftp.c @@ -321,8 +321,8 @@ apr_status_t ap_proxy_send_dir_filter(ap_filter_t *f, apr_bucket_brigade *in) "\n\n\n" "\n\n

Directory of " "%s/", - site, ap_escape_uri(p, path), - site, ap_escape_html(p, path), site); + site, ap_escape_html(p, path), + site, ap_escape_uri(p, path), site); e = apr_bucket_pool_create(str, strlen(str), p); APR_BRIGADE_INSERT_TAIL(out, e); @@ -462,7 +462,7 @@ apr_status_t ap_proxy_send_dir_filter(ap_filter_t f, apr_bucket_brigade in) } filename = strrchr(ctx->buffer, ' '); - (filename++) = 0; + (filename++) = '\0'; /* handle filenames with spaces in 'em / if (!strcmp(filename, ".") || !strcmp(filename, "..") || firstfile) { @@ -471,11 +471,11 @@ apr_status_t ap_proxy_send_dir_filter(ap_filter_t f, apr_bucket_brigade in) } else if (searchidx != 0 && ctx->buffer[searchidx] != 0) { (--filename) = ' '; - ctx->buffer[searchidx - 1] = 0; + ctx->buffer[searchidx - 1] = '\0'; filename = &ctx->buffer[searchidx]; } - /* Special handling for '.' and '..' / + / Append a slash to the HREF link for directories / if (!strcmp(filename, ".") || !strcmp(filename, "..") || ctx->buffer[0] == 'd') { str = apr_psprintf(p, "%s %s\n", ap_escape_html(p, ctx->buffer), @@ -492,9 +492,9 @@ apr_status_t ap_proxy_send_dir_filter(ap_filter_t f, apr_bucket_brigade in) / Try a fallback for listings in the format of "ls -s1" / else if (0 == ap_regexec(re, ctx->buffer, 3, re_result, 0)) { - char filename = apr_pstrndup(p, &ctx->buffer[re_result[2].rm_so], re_result[2].rm_eo - re_result[2].rm_so); + filename = apr_pstrndup(p, &ctx->buffer[re_result[2].rm_so], re_result[2].rm_eo - re_result[2].rm_so); - str = ap_pstrcat(p, ap_escape_html(p, apr_pstrndup(p, &ctx->buffer[0], re_result[2].rm_so)), + str = ap_pstrcat(p, ap_escape_html(p, apr_pstrndup(p, ctx->buffer, re_result[2].rm_so)), "", ap_escape_html(p, filename), "\n", NULL); } @@ -640,7 +640,7 @@ static char ftp_get_PWD(request_rec r, conn_rec ftp_ctrl, apr_bucket_brigade case 421: case 550: ap_proxyerror(r, HTTP_BAD_GATEWAY, - "Failed to read CWD on ftp server"); + "Failed to read PWD on ftp server"); break; case 257: { @@ -1056,8 +1056,14 @@ int ap_proxy_ftp_handler(request_rec r, proxy_server_conf conf, len = decodeenc(path); - rc = proxy_ftp_command(apr_pstrcat(p, "CWD ", path, CRLF, NULL), - r, origin, bb, &ftpmessage); + / NOTE: FTP servers do globbing on the path. + * So we need to escape the URI metacharacters. + * In the current implementation, we use shell escaping, because + * it masks all characters which are also dangerous for FTP. + * We could also have extended gen_test_char.c with a special T_ESCAPE_FTP_PATH + / + rc = proxy_ftp_command(apr_pstrcat(p, "CWD ", + ap_escape_shell_cmd(p, path), CRLF, NULL), strp = '/'; /* responses: 250, 421, 500, 501, 502, 530, 550 / / 250 Requested file action okay, completed. / @@ -1371,9 +1377,10 @@ int ap_proxy_ftp_handler(request_rec r, proxy_server_conf conf, / set request; "path" holds last path component / len = decodeenc(path); - / TM - if len == 0 then it must be a directory (you can't RETR nothing) / - - if (len == 0) { + / If len == 0 then it must be a directory (you can't RETR nothing) + * Also, don't allow to RETR by wildcard. Instead, create a dirlisting + / + if (len == 0 || strpbrk(path, "?") != NULL) { dirlisting = 1; } else { @@ -1395,7 +1402,8 @@ int ap_proxy_ftp_handler(request_rec r, proxy_server_conf conf, ap_log_error(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r->server, "proxy: FTP: SIZE shows this is a directory"); dirlisting = 1; - rc = proxy_ftp_command(apr_pstrcat(p, "CWD ", path, CRLF, NULL), + rc = proxy_ftp_command(apr_pstrcat(p, "CWD ", + ap_escape_shell_cmd(p, path), CRLF, NULL), r, origin, bb, &ftpmessage); /* possible results: 250, 421, 500, 501, 502, 530, 550 / / 250 Requested file action okay, completed. / @@ -1428,9 +1436,12 @@ int ap_proxy_ftp_handler(request_rec r, proxy_server_conf conf, if (dirlisting) { / If the current directory contains no slash, we are talking to * a non-unix ftp system. Try LIST instead of "LIST -lag", it - * should return a long listing anyway (unlink NLST). + * should return a long listing anyway (unlike NLST). * Some exotic FTP servers might choke on the "-lag" switch. / + / Note that we do not escape the path here, to allow for + * queries like: ftp://user@host/apache/src/server/http_.c + / if (len != 0) buf = apr_pstrcat(p, "LIST ", path, CRLF, NULL); else if (cwd == NULL || strchr(cwd, '/') != NULL) @@ -1442,7 +1453,7 @@ int ap_proxy_ftp_handler(request_rec r, proxy_server_conf conf, /* switch to binary if the user did not specify ";type=a" / ftp_set_TYPE(xfer_type, r, origin, bb, &ftpmessage); / FIXME: Handle range requests - send REST / - buf = apr_pstrcat(p, "RETR ", path, CRLF, NULL); + buf = apr_pstrcat(p, "RETR ", ap_escape_shell_cmd(p, path), CRLF, NULL); } rc = proxy_ftp_command(buf, r, origin, bb, &ftpmessage); / rc is an intermediate response for the LIST or RETR commands / @@ -1478,7 +1489,8 @@ int ap_proxy_ftp_handler(request_rec r, proxy_server_conf conf, dirlisting = 1; ftp_set_TYPE('A', r, origin, bb, NULL); - rc = proxy_ftp_command(apr_pstrcat(p, "CWD ", path, CRLF, NULL), + rc = proxy_ftp_command(apr_pstrcat(p, "CWD ", + ap_escape_shell_cmd(p, path), CRLF, NULL), r, origin, bb, &ftpmessage); / possible results: 250, 421, 500, 501, 502, 530, 550 / / 250 Requested file action okay, completed. */