diff options
author | Simon Tatham <anakin@pobox.com> | 2019-05-15 16:57:06 +0300 |
---|---|---|
committer | Simon Tatham <anakin@pobox.com> | 2019-07-10 22:47:09 +0300 |
commit | 70fd577e40d8b16fd3ba6aaf0c0c332af72be684 (patch) | |
tree | 4402162f7dafea97bebc99eb8685269fcd56932d /psftp.c | |
parent | dbd9a07fd089c81b6470b9cc7d33bc9040b0da78 (diff) |
Fall back to not sorting large dirs in pscp -ls or psftp 'ls'.
This mitigates a borderline-DoS in which a malicious SFTP server sends
a ludicrously large number of file names in response to a SFTP
opendir/readdir request sequence, causing the client to buffer them
all and use up all the system's memory simply so that it can produce
the output in sorted order.
I call it a 'borderline' DoS because it's very likely that this is the
same server that you'll also trust to actually send you the _contents_
of some entire file or directory, in which case, if they want to DoS
you they can do that anyway at that point and you have no way to tell
a legit very large file from a bad one. So it's unclear to me that
anyone would get any real advantage out of 'exploiting' this that they
couldn't have got anyway by other means.
That said, it may have practical benefits in the occasional case.
Imagine a _legit_ gigantic directory (something like a maildir,
perhaps, and perhaps stored on a server-side filesystem specialising
in not choking on really huge single directories), together with a
client workflow that involves listing the whole directory but then
downloading only one particular file in it.
For the moment, the threshold size is fixed at 8Mb of total data
(counting the lengths of the file names as well as just the number of
files). If that needs to become configurable later, we can always add
an option.
Diffstat (limited to 'psftp.c')
-rw-r--r-- | psftp.c | 54 |
1 files changed, 17 insertions, 37 deletions
@@ -211,20 +211,6 @@ char *canonify(const char *name) } } -/* - * qsort comparison routine for fxp_name structures. Sorts by real - * file name. - */ -static int sftp_name_compare(const void *av, const void *bv) -{ - const struct fxp_name *const *a = (const struct fxp_name *const *) av; - const struct fxp_name *const *b = (const struct fxp_name *const *) bv; - return strcmp((*a)->filename, (*b)->filename); -} - -/* - * Likewise, but for a bare char *. - */ static int bare_name_compare(const void *av, const void *bv) { const char **a = (const char **) av; @@ -1024,6 +1010,17 @@ int sftp_cmd_close(struct sftp_command *cmd) return 0; } +void list_directory_from_sftp_warn_unsorted(void) +{ + printf("Directory is too large to sort; writing file names unsorted\n"); +} + +void list_directory_from_sftp_print(struct fxp_name *name) +{ + with_stripctrl(san, name->longname) + printf("%s\n", san); +} + /* * List a directory. If no arguments are given, list pwd; otherwise * list the directory given in words[1]. @@ -1032,8 +1029,6 @@ int sftp_cmd_ls(struct sftp_command *cmd) { struct fxp_handle *dirh; struct fxp_names *names; - struct fxp_name **ournames; - size_t nnames, namesize; const char *dir; char *cdir, *unwcdir, *wildcard; struct sftp_packet *pktin; @@ -1091,8 +1086,8 @@ int sftp_cmd_ls(struct sftp_command *cmd) sfree(unwcdir); return 0; } else { - nnames = namesize = 0; - ournames = NULL; + struct list_directory_from_sftp_ctx *ctx = + list_directory_from_sftp_new(); while (1) { @@ -1111,34 +1106,19 @@ int sftp_cmd_ls(struct sftp_command *cmd) break; } - sgrowarrayn(ournames, namesize, nnames, names->nnames); - for (size_t i = 0; i < names->nnames; i++) if (!wildcard || wc_match(wildcard, names->names[i].filename)) - ournames[nnames++] = fxp_dup_name(&names->names[i]); + list_directory_from_sftp_feed(ctx, &names->names[i]); fxp_free_names(names); } + req = fxp_close_send(dirh); pktin = sftp_wait_for_reply(req); fxp_close_recv(pktin, req); - /* - * Now we have our filenames. Sort them by actual file - * name, and then output the longname parts. - */ - if (nnames > 0) - qsort(ournames, nnames, sizeof(*ournames), sftp_name_compare); - - /* - * And print them. - */ - for (size_t i = 0; i < nnames; i++) { - with_stripctrl(san, ournames[i]->longname) - printf("%s\n", san); - fxp_free_name(ournames[i]); - } - sfree(ournames); + list_directory_from_sftp_finish(ctx); + list_directory_from_sftp_free(ctx); } sfree(cdir); |