diff --git a/Recipe b/Recipe index 99efcd2f..6cb794bf 100644 --- a/Recipe +++ b/Recipe @@ -281,7 +281,7 @@ WINSSH = SSH winnoise wincapi winpgntc wingss winshare winnps winnpc UXSSH = SSH uxnoise uxagentc uxgss uxshare # SFTP implementation (pscp, psftp). -SFTP = sftp sftpcommon logging cmdline +SFTP = sftp sftpcommon logging cmdline stripctrl # Miscellaneous objects appearing in all the utilities, or all the # network ones, or the Unix or Windows subsets of those in turn. diff --git a/doc/pscp.but b/doc/pscp.but index 7b90810b..3d520bb6 100644 --- a/doc/pscp.but +++ b/doc/pscp.but @@ -63,6 +63,7 @@ use PSCP: \c -hostkey aa:bb:cc:... \c manually specify a host key (may be repeated) \c -batch disable all interactive prompts +\c -no-sanitise-stderr don't strip control chars from standard error \c -proxycmd command \c use 'command' as local proxy \c -unsafe allow server-side wildcards (DANGEROUS) @@ -281,6 +282,15 @@ The \c{-sftp} option forces PSCP to use the SFTP protocol or quit. When this option is specified, PSCP looks harder for an SFTP server, which may allow use of SFTP with SSH-1 depending on server setup. +\S2{pscp-option-sanitise} \I{-sanitise-stderr}\I{-no-sanitise-stderr}\c{-no-sanitise-stderr}: control error message sanitisation + +The \c{-no-sanitise-stderr} option will cause PSCP to pass through the +server's standard-error stream literally, without stripping control +characters from it first. This might be useful if the server were +sending coloured error messages, but it also gives the server the +ability to have unexpected effects on your terminal display. For more +discussion, see \k{plink-option-sanitise}. + \S{pscp-retval} \ii{Return value} PSCP returns an \i\cw{ERRORLEVEL} of zero (success) only if the files diff --git a/doc/psftp.but b/doc/psftp.but index 93bef742..cc4bb259 100644 --- a/doc/psftp.but +++ b/doc/psftp.but @@ -135,6 +135,15 @@ This may help PSFTP's behaviour when it is used in automated scripts: using \c{-batch}, if something goes wrong at connection time, the batch job will fail rather than hang. +\S2{psftp-option-sanitise} \I{-sanitise-stderr}\I{-no-sanitise-stderr}\c{-no-sanitise-stderr}: control error message sanitisation + +The \c{-no-sanitise-stderr} option will cause PSFTP to pass through the +server's standard-error stream literally, without stripping control +characters from it first. This might be useful if the server were +sending coloured error messages, but it also gives the server the +ability to have unexpected effects on your terminal display. For more +discussion, see \k{plink-option-sanitise}. + \H{psftp-commands} Running PSFTP Once you have started your PSFTP session, you will see a \c{psftp>} diff --git a/pscp.c b/pscp.c index 2abf901e..4592ccf1 100644 --- a/pscp.c +++ b/pscp.c @@ -145,17 +145,16 @@ void agent_schedule_callback(void (*callback)(void *, void *, int), */ static bufchain received_data; +static BinarySink *stderr_bs; static size_t pscp_output( Seat *seat, bool is_stderr, const void *data, size_t len) { /* - * stderr data is just spouted to local stderr and otherwise - * ignored. + * stderr data is just spouted to local stderr (optionally via a + * sanitiser) and otherwise ignored. */ if (is_stderr) { - if (len > 0) - if (fwrite(data, 1, len, stderr) < len) - /* oh well */; + put_data(stderr_bs, data, len); return 0; } @@ -251,6 +250,15 @@ static NORETURN void bump(const char *fmt, ...) cleanup_exit(1); } +/* + * A nasty loop macro that lets me get an escape-sequence sanitised + * version of a string for display, and free it automatically + * afterwards. + */ +#define with_stripctrl(varname, input) \ + for (char *varname = stripctrl_string(input); varname; \ + sfree(varname), varname = NULL) + /* * Wait for the reply to a single SFTP request. Parallels the same * function in psftp.c (but isn't centralised into sftp.c because the @@ -664,8 +672,10 @@ void scp_sftp_listdir(const char *dirname) /* * And print them. */ - for (i = 0; i < nnames; i++) - printf("%s\n", ournames[i].longname); + for (i = 0; i < nnames; i++) { + with_stripctrl(san, ournames[i].longname) + printf("%s\n", san); + } sfree(ournames); } @@ -1175,8 +1185,9 @@ int scp_get_sink_action(struct scp_sink_action *act) ret = fxp_stat_recv(pktin, req, &attrs); if (!ret || !(attrs.flags & SSH_FILEXFER_ATTR_PERMISSIONS)) { - tell_user(stderr, "unable to identify %s: %s", fname, - ret ? "file type not supplied" : fxp_error()); + with_stripctrl(san, fname) + tell_user(stderr, "unable to identify %s: %s", san, + ret ? "file type not supplied" : fxp_error()); if (must_free_fname) sfree(fname); errs++; return 1; @@ -1202,7 +1213,8 @@ int scp_get_sink_action(struct scp_sink_action *act) * things matching the wildcard. */ if (!scp_sftp_recursive && !scp_sftp_wildcard) { - tell_user(stderr, "pscp: %s: is a directory", fname); + with_stripctrl(san, fname) + tell_user(stderr, "pscp: %s: is a directory", san); errs++; if (must_free_fname) sfree(fname); if (scp_sftp_dirstack_head) { @@ -1231,8 +1243,9 @@ int scp_get_sink_action(struct scp_sink_action *act) dirhandle = fxp_opendir_recv(pktin, req); if (!dirhandle) { - tell_user(stderr, "pscp: unable to open directory %s: %s", - fname, fxp_error()); + with_stripctrl(san, fname) + tell_user(stderr, "pscp: unable to open directory %s: %s", + san, fxp_error()); if (must_free_fname) sfree(fname); errs++; return 1; @@ -1249,8 +1262,9 @@ int scp_get_sink_action(struct scp_sink_action *act) if (names == NULL) { if (fxp_error_type() == SSH_FX_EOF) break; - tell_user(stderr, "pscp: reading directory %s: %s", - fname, fxp_error()); + with_stripctrl(san, fname) + tell_user(stderr, "pscp: reading directory %s: %s", + san, fxp_error()); req = fxp_close_send(dirhandle); pktin = sftp_wait_for_reply(req); @@ -1278,9 +1292,9 @@ int scp_get_sink_action(struct scp_sink_action *act) * complaining about. */ } else if (!vet_filename(names->names[i].filename)) { - tell_user(stderr, "ignoring potentially dangerous server-" - "supplied filename '%s'", - names->names[i].filename); + with_stripctrl(san, names->names[i].filename) + tell_user(stderr, "ignoring potentially dangerous " + "server-supplied filename '%s'", san); } else ournames[nnames++] = names->names[i]; } @@ -1382,11 +1396,13 @@ int scp_get_sink_action(struct scp_sink_action *act) act->buf[i - 1] = '\0'; switch (action) { case '\01': /* error */ - tell_user(stderr, "%s", act->buf); + with_stripctrl(san, act->buf) + tell_user(stderr, "%s", san); errs++; continue; /* go round again */ case '\02': /* fatal error */ - bump("%s", act->buf); + with_stripctrl(san, act->buf) + bump("%s", san); case 'E': backend_send(backend, "", 1); act->action = SCP_SINK_ENDDIR; @@ -1442,8 +1458,9 @@ int scp_accept_filexfer(void) scp_sftp_filehandle = fxp_open_recv(pktin, req); if (!scp_sftp_filehandle) { - tell_user(stderr, "pscp: unable to open %s: %s", - scp_sftp_currentname, fxp_error()); + with_stripctrl(san, scp_sftp_currentname) + tell_user(stderr, "pscp: unable to open %s: %s", + san, fxp_error()); errs++; return 1; } @@ -1787,10 +1804,14 @@ static void sink(const char *targ, const char *src) striptarget = stripslashes(act.name, true); if (striptarget != act.name) { - tell_user(stderr, "warning: remote host sent a compound" - " pathname '%s'", act.name); - tell_user(stderr, " renaming local file to '%s'", - striptarget); + with_stripctrl(sanname, act.name) { + with_stripctrl(santarg, act.name) { + tell_user(stderr, "warning: remote host sent a" + " compound pathname '%s'", sanname); + tell_user(stderr, " renaming local", + " file to '%s'", santarg); + } + } } /* @@ -1807,8 +1828,9 @@ static void sink(const char *targ, const char *src) stripsrc = stripslashes(src, true); if (strcmp(striptarget, stripsrc) && !using_sftp && !scp_unsafe_mode) { - tell_user(stderr, "warning: remote host tried to write " - "to a file called '%s'", striptarget); + with_stripctrl(san, striptarget) + tell_user(stderr, "warning: remote host tried to " + "write to a file called '%s'", san); tell_user(stderr, " when we requested a file " "called '%s'.", stripsrc); tell_user(stderr, " If this is a wildcard, " @@ -1837,13 +1859,15 @@ static void sink(const char *targ, const char *src) if (act.action == SCP_SINK_DIR) { if (exists && attr != FILE_TYPE_DIRECTORY) { - run_err("%s: Not a directory", destfname); + with_stripctrl(san, destfname) + run_err("%s: Not a directory", san); sfree(destfname); continue; } if (!exists) { if (!create_directory(destfname)) { - run_err("%s: Cannot create directory", destfname); + with_stripctrl(san, destfname) + run_err("%s: Cannot create directory", san); sfree(destfname); continue; } @@ -1856,7 +1880,8 @@ static void sink(const char *targ, const char *src) f = open_new_file(destfname, act.permissions); if (f == NULL) { - run_err("%s: Cannot create file", destfname); + with_stripctrl(san, destfname) + run_err("%s: Cannot create file", san); sfree(destfname); continue; } @@ -1870,7 +1895,7 @@ static void sink(const char *targ, const char *src) stat_bytes = 0; stat_starttime = time(NULL); stat_lasttime = 0; - stat_name = stripslashes(destfname, true); + stat_name = stripctrl_string(stripslashes(destfname, true)); received = 0; while (received < act.size) { @@ -1914,11 +1939,13 @@ static void sink(const char *targ, const char *src) close_wfile(f); if (wrerror) { - run_err("%s: Write error", destfname); + with_stripctrl(san, destfname) + run_err("%s: Write error", san); sfree(destfname); continue; } (void) scp_finish_filerecv(); + sfree(stat_name); sfree(destfname); sfree(act.buf); } @@ -2133,8 +2160,13 @@ static void get_dir_list(int argc, char *argv[]) if (using_sftp) { scp_sftp_listdir(src); } else { - while (ssh_scp_recv(&c, 1)) - tell_char(stdout, c); + stdio_sink ss; + stdio_sink_init(&ss, stdout); + StripCtrlChars *scc = stripctrl_new( + BinarySink_UPCAST(&ss), false, L'\0'); + while (ssh_scp_recv(&c, 1)) + put_byte(scc, c); + stripctrl_free(scc); } } @@ -2169,9 +2201,13 @@ static void usage(void) printf(" -hostkey aa:bb:cc:...\n"); printf(" manually specify a host key (may be repeated)\n"); printf(" -batch disable all interactive prompts\n"); + printf(" -no-sanitise-stderr don't strip control chars from" + " standard error\n"); printf(" -proxycmd command\n"); printf(" use 'command' as local proxy\n"); printf(" -unsafe allow server-side wildcards (DANGEROUS)\n"); + printf(" -no-sanitise-stderr allow escape sequences in error " + "messages\n"); printf(" -sftp force use of SFTP protocol\n"); printf(" -scp force use of SCP protocol\n"); printf(" -sshlog file\n"); @@ -2202,6 +2238,9 @@ void cmdline_error(const char *p, ...) const bool share_can_be_downstream = true; const bool share_can_be_upstream = false; +static stdio_sink stderr_ss; +static StripCtrlChars *stderr_scc; + /* * Main program. (Called `psftp_main' because it gets called from * *sftp.c; bit silly, I know, but it had to be called _something_.) @@ -2209,6 +2248,7 @@ const bool share_can_be_upstream = false; int psftp_main(int argc, char *argv[]) { int i; + bool sanitise_stderr = true; default_protocol = PROT_TELNET; @@ -2264,6 +2304,10 @@ int psftp_main(int argc, char *argv[]) try_scp = false; try_sftp = true; } else if (strcmp(argv[i], "-scp") == 0) { try_scp = true; try_sftp = false; + } else if (strcmp(argv[i], "-sanitise-stderr") == 0) { + sanitise_stderr = true; + } else if (strcmp(argv[i], "-no-sanitise-stderr") == 0) { + sanitise_stderr = false; } else if (strcmp(argv[i], "--") == 0) { i++; break; @@ -2275,6 +2319,13 @@ int psftp_main(int argc, char *argv[]) argv += i; backend = NULL; + stdio_sink_init(&stderr_ss, stderr); + stderr_bs = BinarySink_UPCAST(&stderr_ss); + if (sanitise_stderr) { + stderr_scc = stripctrl_new(stderr_bs, false, L'\0'); + stderr_bs = BinarySink_UPCAST(stderr_scc); + } + if (list) { if (argc != 1) usage(); diff --git a/psftp.c b/psftp.c index 2e72ce07..0b0228a9 100644 --- a/psftp.c +++ b/psftp.c @@ -64,6 +64,15 @@ static const SeatVtable psftp_seat_vt = { }; static Seat psftp_seat[1] = {{ &psftp_seat_vt }}; +/* ---------------------------------------------------------------------- + * A nasty loop macro that lets me get an escape-sequence sanitised + * version of a string for display, and free it automatically + * afterwards. + */ +#define with_stripctrl(varname, input) \ + for (char *varname = stripctrl_string(input); varname; \ + sfree(varname), varname = NULL) + /* ---------------------------------------------------------------------- * Manage sending requests and waiting for replies. */ @@ -266,7 +275,8 @@ bool sftp_get_file(char *fname, char *outfname, bool recurse, bool restart) */ if (file_type(outfname) != FILE_TYPE_DIRECTORY && !create_directory(outfname)) { - printf("%s: Cannot create directory\n", outfname); + with_stripctrl(san, outfname) + printf("%s: Cannot create directory\n", san); return false; } @@ -279,8 +289,9 @@ bool sftp_get_file(char *fname, char *outfname, bool recurse, bool restart) dirhandle = fxp_opendir_recv(pktin, req); if (!dirhandle) { - printf("%s: unable to open directory: %s\n", - fname, fxp_error()); + with_stripctrl(san, fname) + printf("%s: unable to open directory: %s\n", + san, fxp_error()); return false; } nnames = namesize = 0; @@ -295,7 +306,9 @@ bool sftp_get_file(char *fname, char *outfname, bool recurse, bool restart) if (names == NULL) { if (fxp_error_type() == SSH_FX_EOF) break; - printf("%s: reading directory: %s\n", fname, fxp_error()); + with_stripctrl(san, fname) + printf("%s: reading directory: %s\n", + san, fxp_error()); req = fxp_close_send(dirhandle); pktin = sftp_wait_for_reply(req); @@ -316,9 +329,9 @@ bool sftp_get_file(char *fname, char *outfname, bool recurse, bool restart) if (strcmp(names->names[i].filename, ".") && strcmp(names->names[i].filename, "..")) { if (!vet_filename(names->names[i].filename)) { - printf("ignoring potentially dangerous server-" - "supplied filename '%s'\n", - names->names[i].filename); + with_stripctrl(san, names->names[i].filename) + printf("ignoring potentially dangerous server-" + "supplied filename '%s'\n", san); } else { ournames[nnames++] = fxp_dup_name(&names->names[i]); @@ -414,7 +427,8 @@ bool sftp_get_file(char *fname, char *outfname, bool recurse, bool restart) fh = fxp_open_recv(pktin, req); if (!fh) { - printf("%s: open for read: %s\n", fname, fxp_error()); + with_stripctrl(san, fname) + printf("%s: open for read: %s\n", san, fxp_error()); return false; } @@ -425,7 +439,8 @@ bool sftp_get_file(char *fname, char *outfname, bool recurse, bool restart) } if (!file) { - printf("local: unable to open %s\n", outfname); + with_stripctrl(san, outfname) + printf("local: unable to open %s\n", san); req = fxp_close_send(fh); pktin = sftp_wait_for_reply(req); @@ -437,8 +452,8 @@ bool sftp_get_file(char *fname, char *outfname, bool recurse, bool restart) if (restart) { if (seek_file(file, 0, FROM_END) == -1) { close_wfile(file); - printf("reget: cannot restart %s - file too large\n", - outfname); + with_stripctrl(san, outfname) + printf("reget: cannot restart %s - file too large\n", san); req = fxp_close_send(fh); pktin = sftp_wait_for_reply(req); fxp_close_recv(pktin, req); @@ -452,7 +467,10 @@ bool sftp_get_file(char *fname, char *outfname, bool recurse, bool restart) offset = 0; } - printf("remote:%s => local:%s\n", fname, outfname); + with_stripctrl(san, fname) { + with_stripctrl(sano, outfname) + printf("remote:%s => local:%s\n", san, sano); + } /* * FIXME: we can use FXP_FSTAT here to get the file size, and @@ -839,9 +857,11 @@ char *sftp_wildcard_get_filename(SftpWildcardMatcher *swcm) swcm->names = fxp_readdir_recv(pktin, req); if (!swcm->names) { - if (fxp_error_type() != SSH_FX_EOF) - printf("%s: reading directory: %s\n", swcm->prefix, - fxp_error()); + if (fxp_error_type() != SSH_FX_EOF) { + with_stripctrl(san, swcm->prefix) + printf("%s: reading directory: %s\n", + san, fxp_error()); + } return NULL; } else if (swcm->names->nnames == 0) { /* @@ -866,8 +886,9 @@ char *sftp_wildcard_get_filename(SftpWildcardMatcher *swcm) continue; /* expected bad filenames */ if (!vet_filename(name->filename)) { - printf("ignoring potentially dangerous server-" - "supplied filename '%s'\n", name->filename); + with_stripctrl(san, name->filename) + printf("ignoring potentially dangerous server-" + "supplied filename '%s'\n", san); continue; /* unexpected bad filename */ } @@ -1060,7 +1081,8 @@ int sftp_cmd_ls(struct sftp_command *cmd) cdir = canonify(dir); - printf("Listing directory %s\n", cdir); + with_stripctrl(san, cdir) + printf("Listing directory %s\n", san); req = fxp_opendir_send(cdir); pktin = sftp_wait_for_reply(req); @@ -1118,7 +1140,8 @@ int sftp_cmd_ls(struct sftp_command *cmd) * And print them. */ for (i = 0; i < nnames; i++) { - printf("%s\n", ournames[i]->longname); + with_stripctrl(san, ournames[i]->longname) + printf("%s\n", san); fxp_free_name(ournames[i]); } sfree(ournames); @@ -1157,7 +1180,8 @@ int sftp_cmd_cd(struct sftp_command *cmd) dirh = fxp_opendir_recv(pktin, req); if (!dirh) { - printf("Directory %s: %s\n", dir, fxp_error()); + with_stripctrl(san, dir) + printf("Directory %s: %s\n", san, fxp_error()); sfree(dir); return 0; } @@ -1168,7 +1192,8 @@ int sftp_cmd_cd(struct sftp_command *cmd) sfree(pwd); pwd = dir; - printf("Remote directory is now %s\n", pwd); + with_stripctrl(san, pwd) + printf("Remote directory is now %s\n", san); return 1; } @@ -1183,7 +1208,8 @@ int sftp_cmd_pwd(struct sftp_command *cmd) return 0; } - printf("Remote directory is %s\n", pwd); + with_stripctrl(san, pwd) + printf("Remote directory is %s\n", san); return 1; } @@ -1423,10 +1449,12 @@ int sftp_cmd_mkdir(struct sftp_command *cmd) result = fxp_mkdir_recv(pktin, req); if (!result) { - printf("mkdir %s: %s\n", dir, fxp_error()); + with_stripctrl(san, dir) + printf("mkdir %s: %s\n", san, fxp_error()); ret = 0; } else - printf("mkdir %s: OK\n", dir); + with_stripctrl(san, dir) + printf("mkdir %s: OK\n", san); sfree(dir); } @@ -1571,10 +1599,12 @@ static bool sftp_action_mv(void *vctx, char *srcfname) error = result ? NULL : fxp_error(); if (error) { - printf("mv %s %s: %s\n", srcfname, finalfname, error); + with_stripctrl(san, finalfname) + printf("mv %s %s: %s\n", srcfname, san, error); toret = false; } else { - printf("%s -> %s\n", srcfname, finalfname); + with_stripctrl(san, finalfname) + printf("%s -> %s\n", srcfname, san); toret = true; } @@ -2332,7 +2362,8 @@ static int do_sftp_init(void) fxp_error()); homedir = dupstr("."); } else { - printf("Remote working directory is %s\n", homedir); + with_stripctrl(san, homedir) + printf("Remote working directory is %s\n", san); } pwd = dupstr(homedir); return 0; @@ -2444,19 +2475,17 @@ void agent_schedule_callback(void (*callback)(void *, void *, int), * own psftp_output() function to catch the data that comes back. We * do this until we have enough data. */ - static bufchain received_data; +static BinarySink *stderr_bs; static size_t psftp_output( Seat *seat, bool is_stderr, const void *data, size_t len) { /* - * stderr data is just spouted to local stderr and otherwise - * ignored. + * stderr data is just spouted to local stderr (optionally via a + * sanitiser) and otherwise ignored. */ if (is_stderr) { - if (len > 0) - if (fwrite(data, 1, len, stderr) < len) - /* oh well */; + put_data(stderr_bs, data, len); return 0; } @@ -2532,8 +2561,12 @@ static void usage(void) printf(" -hostkey aa:bb:cc:...\n"); printf(" manually specify a host key (may be repeated)\n"); printf(" -batch disable all interactive prompts\n"); + printf(" -no-sanitise-stderr don't strip control chars from" + " standard error\n"); printf(" -proxycmd command\n"); printf(" use 'command' as local proxy\n"); + printf(" -no-sanitise-stderr allow escape sequences in error " + "messages\n"); printf(" -sshlog file\n"); printf(" -sshrawlog file\n"); printf(" log protocol details to a file\n"); @@ -2754,6 +2787,9 @@ void cmdline_error(const char *p, ...) const bool share_can_be_downstream = true; const bool share_can_be_upstream = false; +static stdio_sink stderr_ss; +static StripCtrlChars *stderr_scc; + /* * Main program. Parse arguments etc. */ @@ -2764,6 +2800,7 @@ int psftp_main(int argc, char *argv[]) char *userhost, *user; int mode = 0; int modeflags = 0; + bool sanitise_stderr = true; char *batchfile = NULL; flags = FLAG_INTERACTIVE @@ -2818,6 +2855,10 @@ int psftp_main(int argc, char *argv[]) modeflags = modeflags | 1; } else if (strcmp(argv[i], "-be") == 0) { modeflags = modeflags | 2; + } else if (strcmp(argv[i], "-sanitise-stderr") == 0) { + sanitise_stderr = true; + } else if (strcmp(argv[i], "-no-sanitise-stderr") == 0) { + sanitise_stderr = false; } else if (strcmp(argv[i], "--") == 0) { i++; break; @@ -2829,6 +2870,13 @@ int psftp_main(int argc, char *argv[]) argv += i; backend = NULL; + stdio_sink_init(&stderr_ss, stderr); + stderr_bs = BinarySink_UPCAST(&stderr_ss); + if (sanitise_stderr) { + stderr_scc = stripctrl_new(stderr_bs, false, L'\0'); + stderr_bs = BinarySink_UPCAST(stderr_scc); + } + /* * If the loaded session provides a hostname, and a hostname has not * otherwise been specified, pop it in `userhost' so that