diff --git a/src/document/html/parser/parse.c b/src/document/html/parser/parse.c index 46e636f8f..135f64599 100644 --- a/src/document/html/parser/parse.c +++ b/src/document/html/parser/parse.c @@ -211,7 +211,7 @@ found_endattr: mem_free(saved_attr); } - set_mem_comment(trim_chars(attr, ' ', NULL), name, strlen(name)); + set_mem_comment(attr, name, strlen(name)); return attr; } else { diff --git a/src/protocol/common.c b/src/protocol/common.c index c21dc42ca..d22fd84b1 100644 --- a/src/protocol/common.c +++ b/src/protocol/common.c @@ -47,12 +47,14 @@ enum connection_state init_directory_listing(struct string *page, struct uri *uri) { struct string dirpath = NULL_STRING; + struct string decoded = NULL_STRING; struct string location = NULL_STRING; unsigned char *info; int local = (uri->protocol == PROTOCOL_FILE); if (!init_string(page) || !init_string(&dirpath) + || !init_string(&decoded) || !init_string(&location) || !add_uri_to_string(&dirpath, uri, URI_DATA) || !add_uri_to_string(&location, uri, URI_DIR_LOCATION)) @@ -63,12 +65,12 @@ init_directory_listing(struct string *page, struct uri *uri) && !add_char_to_string(&dirpath, local ? CHAR_DIR_SEP : '/')) goto out_of_memory; - if (local || uri->protocol == PROTOCOL_FSP || uri->protocol == PROTOCOL_GOPHER - || uri->protocol == PROTOCOL_SMB) { - /* A little hack to get readable Gopher names. We should find a - * way to do it more general. */ - decode_uri_string(&dirpath); - } + /* Decode uri for displaying. Do not use + * add_string_to_string, because it for some reason returns + * NULL if the second string is empty. */ + if (!add_bytes_to_string(&decoded, dirpath.source, dirpath.length)) + goto out_of_memory; + decode_uri_string(&decoded); if (!local && !add_char_to_string(&location, '/')) goto out_of_memory; @@ -76,23 +78,15 @@ init_directory_listing(struct string *page, struct uri *uri) if (!add_to_string(page, "\n")) goto out_of_memory; - if (!local && !add_string_to_string(page, &location)) + if (!local && !add_html_to_string(page, location.source, location.length)) goto out_of_memory; - if (!add_html_to_string(page, dirpath.source, dirpath.length) + if (!add_html_to_string(page, decoded.source, decoded.length) || !add_to_string(page, "\n\n\n\n

")) goto out_of_memory; @@ -126,20 +120,24 @@ init_directory_listing(struct string *page, struct uri *uri) /* Make the directory path with links to each subdir. */ { - unsigned char *slash = dirpath.source; - unsigned char *pslash = slash; - unsigned char sep = local ? CHAR_DIR_SEP : '/'; + const unsigned char *slash = dirpath.source; + const unsigned char *pslash = slash; + const unsigned char sep = local ? CHAR_DIR_SEP : '/'; + + while ((slash = strchr(slash, sep)) != NULL) { + done_string(&decoded); + if (!init_string(&decoded) + || !add_bytes_to_string(&decoded, pslash, slash - pslash)) + goto out_of_memory; + decode_uri_string(&decoded); - while ((slash = strchr(slash, sep))) { - /* FIXME: htmlesc? At least we should escape quotes. --pasky */ if (!add_to_string(page, "") - || !add_html_to_string(page, pslash, slash - pslash) + || !add_html_to_string(page, decoded.source, decoded.length) || !add_to_string(page, "") - || !add_char_to_string(page, sep)) + || !add_html_to_string(page, &sep, 1)) goto out_of_memory; pslash = ++slash; @@ -152,6 +150,7 @@ out_of_memory: } done_string(&dirpath); + done_string(&decoded); done_string(&location); return page->length > 0 ? S_OK : S_OUT_OF_MEM; diff --git a/src/protocol/ftp/ftp.c b/src/protocol/ftp/ftp.c index 01e301bc0..044af97ce 100644 --- a/src/protocol/ftp/ftp.c +++ b/src/protocol/ftp/ftp.c @@ -638,6 +638,26 @@ get_ftp_data_socket(struct connection *conn, struct string *command) return 1; } +/* Check if the file or directory name @s can be safely sent to the + * FTP server. To prevent command injection attacks, this function + * must reject CR LF sequences. */ +static int +is_ftp_pathname_safe(const struct string *s) +{ + int i; + + /* RFC 959 says the argument of CWD and RETR is a , + * which consists of s, "any of the 128 ASCII characters + * except and ". So other control characters, such + * as 0x00 and 0x7F, are allowed here. Bytes 0x80...0xFF + * should not be allowed, but if we reject them, users will + * probably complain. */ + for (i = 0; i < s->length; i++) { + if (s->source[i] == 0x0A || s->source[i] == 0x0D) + return 0; + } + return 1; +} /* Create passive socket and add appropriate announcing commands to str. Then * go and retrieve appropriate object from server. @@ -645,40 +665,53 @@ get_ftp_data_socket(struct connection *conn, struct string *command) static struct ftp_connection_info * add_file_cmd_to_str(struct connection *conn) { - struct ftp_connection_info *ftp; - struct string command, ftp_data_command; + int ok = 0; + struct ftp_connection_info *ftp = NULL; + struct string command = NULL_STRING; + struct string ftp_data_command = NULL_STRING; + struct string pathname = NULL_STRING; if (!conn->uri->data) { INTERNAL("conn->uri->data empty"); abort_connection(conn, S_INTERNAL); - return NULL; + goto ret; } + /* This will be reallocated below when we know how long the + * command string should be. Error handling could be + * simplified a little by allocating this initial structure on + * the stack, but it's several kilobytes long so that might be + * risky. */ ftp = mem_calloc(1, sizeof(*ftp)); if (!ftp) { abort_connection(conn, S_OUT_OF_MEM); - return NULL; + goto ret; } conn->info = ftp; /* Freed when connection is destroyed. */ - if (!init_string(&command)) { + if (!init_string(&command) + || !init_string(&ftp_data_command) + || !init_string(&pathname)) { abort_connection(conn, S_OUT_OF_MEM); - return NULL; - } - - if (!init_string(&ftp_data_command)) { - done_string(&command); - abort_connection(conn, S_OUT_OF_MEM); - return NULL; + goto ret; } if (!get_ftp_data_socket(conn, &ftp_data_command)) { - done_string(&command); - done_string(&ftp_data_command); INTERNAL("Ftp data socket failure"); abort_connection(conn, S_INTERNAL); - return NULL; + goto ret; + } + + if (!add_uri_to_string(&pathname, conn->uri, URI_PATH)) { + abort_connection(conn, S_OUT_OF_MEM); + goto ret; + } + + decode_uri_string(&pathname); + if (!is_ftp_pathname_safe(&pathname)) { + abort_connection(conn, S_BAD_URL); + goto ret; } if (!conn->uri->datalen @@ -688,18 +721,20 @@ add_file_cmd_to_str(struct connection *conn) ftp->dir = 1; ftp->pending_commands = 4; - /* ASCII */ - add_to_string(&command, "TYPE A"); - add_crlf_to_string(&command); + if (!add_to_string(&command, "TYPE A") /* ASCII */ + || !add_crlf_to_string(&command) - add_string_to_string(&command, &ftp_data_command); + || !add_string_to_string(&command, &ftp_data_command) + + || !add_to_string(&command, "CWD ") + || !add_string_to_string(&command, &pathname) + || !add_crlf_to_string(&command) - add_to_string(&command, "CWD "); - add_uri_to_string(&command, conn->uri, URI_PATH); - add_crlf_to_string(&command); - - add_to_string(&command, "LIST"); - add_crlf_to_string(&command); + || !add_to_string(&command, "LIST") + || !add_crlf_to_string(&command)) { + abort_connection(conn, S_OUT_OF_MEM); + goto ret; + } conn->from = 0; @@ -709,45 +744,58 @@ add_file_cmd_to_str(struct connection *conn) ftp->dir = 0; ftp->pending_commands = 3; - /* BINARY */ - add_to_string(&command, "TYPE I"); - add_crlf_to_string(&command); + if (!add_to_string(&command, "TYPE I") /* BINARY */ + || !add_crlf_to_string(&command) - add_string_to_string(&command, &ftp_data_command); + || !add_string_to_string(&command, &ftp_data_command)) { + abort_connection(conn, S_OUT_OF_MEM); + goto ret; + } if (conn->from || conn->progress->start > 0) { - add_to_string(&command, "REST "); - add_long_to_string(&command, conn->from - ? conn->from - : conn->progress->start); - add_crlf_to_string(&command); + const off_t offset = conn->from + ? conn->from + : conn->progress->start; + + if (!add_to_string(&command, "REST ") + || !add_long_to_string(&command, offset) + || !add_crlf_to_string(&command)) { + abort_connection(conn, S_OUT_OF_MEM); + goto ret; + } ftp->rest_sent = 1; ftp->pending_commands++; } - add_to_string(&command, "RETR "); - add_uri_to_string(&command, conn->uri, URI_PATH); - add_crlf_to_string(&command); + if (!add_to_string(&command, "RETR ") + || !add_string_to_string(&command, &pathname) + || !add_crlf_to_string(&command)) { + abort_connection(conn, S_OUT_OF_MEM); + goto ret; + } } - done_string(&ftp_data_command); - ftp->opc = ftp->pending_commands; /* 1 byte is already reserved for cmd_buffer in struct ftp_connection_info. */ ftp = mem_realloc(ftp, sizeof(*ftp) + command.length); if (!ftp) { - done_string(&command); abort_connection(conn, S_OUT_OF_MEM); - return NULL; + goto ret; } + conn->info = ftp; /* in case mem_realloc moved the buffer */ memcpy(ftp->cmd_buffer, command.source, command.length + 1); - done_string(&command); - conn->info = ftp; + ok = 1; - return ftp; +ret: + /* If @ok is false here, then abort_connection has already + * freed @ftp, which now is a dangling pointer. */ + done_string(&pathname); + done_string(&ftp_data_command); + done_string(&command); + return ok ? ftp : NULL; } static void @@ -1134,7 +1182,7 @@ display_dir_entry(struct cache_entry *cached, off_t *pos, int *tries, } add_to_string(&string, "name.source, ftp_info->name.length); + encode_uri_string(&string, ftp_info->name.source, ftp_info->name.length, 0); if (ftp_info->type == FTP_FILE_DIRECTORY) add_char_to_string(&string, '/'); add_to_string(&string, "\">"); diff --git a/src/protocol/uri.c b/src/protocol/uri.c index 634688b5e..df226464c 100644 --- a/src/protocol/uri.c +++ b/src/protocol/uri.c @@ -47,14 +47,14 @@ end_of_dir(unsigned char c) } static inline int -is_uri_dir_sep(struct uri *uri, unsigned char pos) +is_uri_dir_sep(const struct uri *uri, unsigned char pos) { return (uri->protocol == PROTOCOL_FILE ? dir_sep(pos) : pos == '/'); } int -is_ip_address(unsigned char *address, int addresslen) +is_ip_address(const unsigned char *address, int addresslen) { /* The @address has well defined limits so it would be a shame to * allocate it. */ @@ -90,7 +90,7 @@ is_ip_address(unsigned char *address, int addresslen) int -end_with_known_tld(unsigned char *s, int slen) +end_with_known_tld(const unsigned char *s, int slen) { int i; static const unsigned char *const tld[] = @@ -146,7 +146,7 @@ check_whether_file_exists(unsigned char *name) } static int -check_uri_file(unsigned char *name) +check_uri_file(const unsigned char *name) { /* Check POST_CHAR etc ... */ static const unsigned char chars[] = POST_CHAR_S "#?"; @@ -409,10 +409,10 @@ parse_uri(struct uri *uri, unsigned char *uristring) } int -get_uri_port(struct uri *uri) +get_uri_port(const struct uri *uri) { if (uri->port && uri->portlen) { - unsigned char *end = uri->port; + const unsigned char *end = uri->port; int port = strtol(uri->port, (char **) &end, 10); if (end != uri->port) { @@ -427,7 +427,8 @@ get_uri_port(struct uri *uri) #define can_compare_uri_components(comp) !(((comp) & (URI_SPECIAL | URI_IDN))) static inline int -compare_component(unsigned char *a, int alen, unsigned char *b, int blen) +compare_component(const unsigned char *a, int alen, + const unsigned char *b, int blen) { /* Check that the length and the strings are both set or unset */ if (alen != blen || !!a != !!b) return 0; @@ -442,7 +443,8 @@ compare_component(unsigned char *a, int alen, unsigned char *b, int blen) #define wants(x) (components & (x)) int -compare_uri(struct uri *a, struct uri *b, enum uri_component components) +compare_uri(const struct uri *a, const struct uri *b, + enum uri_component components) { if (a == b) return 1; if (!components) return 0; @@ -471,7 +473,7 @@ compare_uri(struct uri *a, struct uri *b, enum uri_component components) /* We might need something more intelligent than this Swiss army knife. */ struct string * -add_uri_to_string(struct string *string, struct uri *uri, +add_uri_to_string(struct string *string, const struct uri *uri, enum uri_component components) { /* Custom or unknown keep the URI untouched. */ @@ -578,8 +580,8 @@ add_uri_to_string(struct string *string, struct uri *uri, /* We can not test uri->datalen here since we need to always * add '/'. */ if (wants(URI_PATH) || wants(URI_FILENAME)) { - unsigned char *filename = uri->data; - unsigned char *pos; + const unsigned char *filename = uri->data; + const unsigned char *pos; assertm(!wants(URI_FILENAME) || components == URI_FILENAME, "URI_FILENAME should be used alone %d", components); @@ -602,7 +604,7 @@ add_uri_to_string(struct string *string, struct uri *uri, } if (wants(URI_QUERY) && uri->datalen) { - unsigned char *query = memchr(uri->data, '?', uri->datalen); + const unsigned char *query = memchr(uri->data, '?', uri->datalen); assertm(URI_QUERY == components, "URI_QUERY should be used alone %d", components); @@ -642,7 +644,7 @@ add_uri_to_string(struct string *string, struct uri *uri, #undef wants unsigned char * -get_uri_string(struct uri *uri, enum uri_component components) +get_uri_string(const struct uri *uri, enum uri_component components) { struct string string; @@ -785,7 +787,7 @@ normalize_uri(struct uri *uri, unsigned char *uristring) * backend can understand. No host parts etc, that is what this function is * supposed to chew. */ static struct uri * -transform_file_url(struct uri *uri, unsigned char *cwd) +transform_file_url(struct uri *uri, const unsigned char *cwd) { unsigned char *path = uri->data; diff --git a/src/protocol/uri.h b/src/protocol/uri.h index b3eeecc4e..426bd11c8 100644 --- a/src/protocol/uri.h +++ b/src/protocol/uri.h @@ -247,25 +247,28 @@ unsigned char *normalize_uri(struct uri *uri, unsigned char *uristring); /* Check if two URIs are equal. If @components are 0 simply compare the whole * URI else only compare the specific parts. */ -int compare_uri(struct uri *uri1, struct uri *uri2, enum uri_component components); +int compare_uri(const struct uri *uri1, const struct uri *uri2, + enum uri_component components); /* These functions recreate the URI string part by part. */ /* The @components bitmask describes the set of URI components used for * construction of the URI string. */ /* Adds the components to an already initialized string. */ -struct string *add_uri_to_string(struct string *string, struct uri *uri, enum uri_component components); +struct string *add_uri_to_string(struct string *string, const struct uri *uri, + enum uri_component components); /* Takes an uri string, parses it and adds the desired components. Useful if * there is no struct uri around. */ struct string *add_string_uri_to_string(struct string *string, unsigned char *uristring, enum uri_component components); /* Returns the new URI string or NULL upon an error. */ -unsigned char *get_uri_string(struct uri *uri, enum uri_component components); +unsigned char *get_uri_string(const struct uri *uri, + enum uri_component components); /* Returns either the uri's port number if available or the protocol's * default port. It is zarro for user protocols. */ -int get_uri_port(struct uri *uri); +int get_uri_port(const struct uri *uri); /* Tcp port range */ #define LOWEST_PORT 0 @@ -304,7 +307,7 @@ unsigned char *join_urls(struct uri *base, unsigned char *relative); /* Return position if end of string @s matches a known tld or -1 if not. * If @slen < 0, then string length will be obtained by a strlen() call, * else @slen is used as @s length. */ -int end_with_known_tld(unsigned char *s, int slen); +int end_with_known_tld(const unsigned char *s, int slen); static inline int @@ -314,6 +317,6 @@ get_real_uri_length(struct uri *uri) } /* Checks if @address contains a valid IP address. */ -int is_ip_address(unsigned char *address, int addresslen); +int is_ip_address(const unsigned char *address, int addresslen); #endif diff --git a/src/util/conv.c b/src/util/conv.c index 5a43dbc65..5beb001f3 100644 --- a/src/util/conv.c +++ b/src/util/conv.c @@ -274,22 +274,26 @@ add_string_replace(struct string *string, unsigned char *src, int len, struct string * add_html_to_string(struct string *string, const unsigned char *src, int len) { - -#define isalphanum(q) (isalnum(q) || (q) == '-' || (q) == '_') - for (; len; len--, src++) { - if (isalphanum(*src) || *src == ' ' - || *src == '.' || *src == ':' || *src == ';') { - add_bytes_to_string(string, src, 1); + if (*src < 0x20 || *src >= 0x7F + || *src == '<' || *src == '>' || *src == '&' + || *src == '\"' || *src == '\'') { + int rollback_length = string->length; + + if (!add_bytes_to_string(string, "&#", 2) + || !add_long_to_string(string, (long) *src) + || !add_char_to_string(string, ';')) { + string->length = rollback_length; + string->source[rollback_length] = '\0'; + return NULL; + } + } else { - add_bytes_to_string(string, "&#", 2); - add_long_to_string(string, (long) *src); - add_char_to_string(string, ';'); + if (!add_char_to_string(string, *src)) + return NULL; } } -#undef isalphanum - return string; } diff --git a/src/viewer/text/link.c b/src/viewer/text/link.c index 7a69a9ce7..2c500074f 100644 --- a/src/viewer/text/link.c +++ b/src/viewer/text/link.c @@ -1197,7 +1197,6 @@ try_document_key(struct session *ses, struct document_view *doc_view, struct term_event *ev) { unicode_val_T key; - int passed = -1; int i; /* GOD I HATE C! --FF */ /* YEAH, BRAINFUCK RULEZ! --pasky */ assert(ses && doc_view && doc_view->document && doc_view->vs && ev); @@ -1224,24 +1223,24 @@ try_document_key(struct session *ses, struct document_view *doc_view, /* Run through all the links and see if one of them is bound to the * key we test.. */ - for (i = 0; i < doc_view->document->nlinks; i++) { + + i = doc_view->vs->current_link + 1; + for (; i < doc_view->document->nlinks; i++) { struct link *link = &doc_view->document->links[i]; if (key == link->accesskey) { - if (passed != i && i <= doc_view->vs->current_link) { - /* This is here in order to rotate between - * links with same accesskey. */ - if (passed < 0) passed = i; - continue; - } ses->kbdprefix.repeat_count = 0; goto_link_number_do(ses, doc_view, i); return FRAME_EVENT_REFRESH; } + } + for (i = 0; i <= doc_view->vs->current_link; i++) { + struct link *link = &doc_view->document->links[i]; - if (i == doc_view->document->nlinks - 1 && passed >= 0) { - /* Return to the start. */ - i = passed - 1; + if (key == link->accesskey) { + ses->kbdprefix.repeat_count = 0; + goto_link_number_do(ses, doc_view, i); + return FRAME_EVENT_REFRESH; } }