On Tue, Mar 10, 2020 at 07:52:17PM +0200, guysv wrote:
> Because ",',<,>,& are all valid unix filename characters,
> filenames containing those characters can glitch-out a dirlist
> response.
>
> A funny example would be:
> "><img src="blabla" onerror="alert(1)"
>
> This commit escapes dynamic input, and fixes the bug.
> ---
> resp.c | 29 +++++++++++++++++++++++++----
> 1 file changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/resp.c b/resp.c
> index 3075c28..70170bc 100644
> --- a/resp.c
> +++ b/resp.c
> _AT_@ -1,5 +1,6 @@
> /* See LICENSE file for copyright and license details. */
> #include <dirent.h>
> +#include <limits.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> _AT_@ -38,12 +39,31 @@ suffix(int t)
> return "";
> }
>
> +void
> +htmlescape(char *src, char *dst)
> +{
> + for (; *src; src++) {
> + switch (*src) {
> + case '<': strcat(dst, "<"); dst += sizeof("<")-1; break;
> + case '>': strcat(dst, ">"); dst += sizeof(">")-1; break;
> + case '&': strcat(dst, "&"); dst += sizeof("&")-1; break;
> + case '"': strcat(dst, """); dst += sizeof(""")-1; break;
> + case '\'': strcat(dst, "'"); dst += sizeof("'")-1; break;
> + default: *dst++ = *src; break;
> + }
> + }
> + *dst = '\0';
> +}
> +
I think its good practise to close HTML entities with semi-colon. The numeric
entity should be closed.
So "&" to "&".
> enum status
> resp_dir(int fd, char *name, struct request *r)
> {
> struct dirent **e;
> size_t i;
> int dirlen, s;
> + /* 5 - strlen("""), largest escape */
> + char escaped_filename[NAME_MAX*5];
> + char escaped_dirname[PATH_MAX*5];
This buffer can be reused saving stack space.
> static char t[TIMESTAMP_LEN];
>
> /* read directory */
> _AT_@ -64,12 +84,13 @@ resp_dir(int fd, char *name, struct request *r)
> }
>
> if (r->method == M_GET) {
> + htmlescape(name, escaped_dirname);
> /* listing header */
> if (dprintf(fd,
> "<!DOCTYPE html>\n<html>\n\t<head>"
> "<title>Index of %s</title></head>\n"
> "\t<body>\n\t\t<a href=\"..\">..</a>",
> - name) < 0) {
> + escaped_dirname) < 0) {
> s = S_REQUEST_TIMEOUT;
> goto cleanup;
> }
> _AT_@ -80,12 +101,12 @@ resp_dir(int fd, char *name, struct request *r)
> if (e[i]->d_name[0] == '.') {
> continue;
> }
> -
> + htmlescape(e[i]->d_name, escaped_filename);
> /* entry line */
> if (dprintf(fd, "<br />\n\t\t<a href=\"%s%s\">%s%s</a>",
> - e[i]->d_name,
> + escaped_filename,
> (e[i]->d_type == DT_DIR) ? "/" : "",
> - e[i]->d_name,
> + escaped_filename,
> suffix(e[i]->d_type)) < 0) {
> s = S_REQUEST_TIMEOUT;
> goto cleanup;
> --
> 2.25.1
>
>
--
Kind regards,
Hiltjo
Received on Wed Mar 11 2020 - 00:42:10 CET