libhtp: harden code against malloc failures. Bug #587.

pull/169/head
Victor Julien 13 years ago
parent da7f1d22cc
commit 76f0838a9f

@ -457,6 +457,8 @@ static void *table_get_internal(table_t *table, bstr *key) {
* @return table element, or NULL if not found
*/
void *table_getc(table_t *table, char *cstr) {
if (table == NULL||cstr == NULL)
return NULL;
// TODO This is very inefficient
bstr *key = bstr_cstrdup(cstr);
if (key == NULL)
@ -475,7 +477,7 @@ void *table_getc(table_t *table, char *cstr) {
* @return table element, or NULL if not found
*/
void *table_get(table_t *table, bstr *key) {
if (table == NULL)
if (table == NULL||key == NULL)
return NULL;
// TODO This is very inefficient
bstr *lkey = bstr_dup_lower(key);

@ -58,25 +58,29 @@ void htp_conn_destroy(htp_conn_t *conn) {
// using the iterator does not work here because some of the
// list element may be NULL (and with the iterator it is impossible
// to distinguish a NULL element from the end of the list).
size_t i;
for (i = 0; i < list_size(conn->transactions); i++) {
htp_tx_t *tx = (htp_tx_t *)list_get(conn->transactions, i);
if (tx != NULL) {
htp_tx_destroy(tx);
if (conn->transactions != NULL) {
size_t i;
for (i = 0; i < list_size(conn->transactions); i++) {
htp_tx_t *tx = (htp_tx_t *)list_get(conn->transactions, i);
if (tx != NULL) {
htp_tx_destroy(tx);
}
}
list_destroy(conn->transactions);
}
list_destroy(conn->transactions);
// Destroy individual messages
htp_log_t *l = NULL;
list_iterator_reset(conn->messages);
while ((l = list_iterator_next(conn->messages)) != NULL) {
free((void *)l->msg);
free(l);
}
if (conn->messages != NULL) {
htp_log_t *l = NULL;
list_iterator_reset(conn->messages);
while ((l = list_iterator_next(conn->messages)) != NULL) {
free((void *)l->msg);
free(l);
}
list_destroy(conn->messages);
list_destroy(conn->messages);
}
if (conn->local_addr != NULL) {
free(conn->local_addr);
@ -101,7 +105,7 @@ void htp_conn_destroy(htp_conn_t *conn) {
* @return 1 if transaction was removed or 0 if it wasn't found
*/
int htp_conn_remove_tx(htp_conn_t *conn, htp_tx_t *tx) {
if ((tx == NULL)||(conn == NULL)) return 0;
if ((tx == NULL)||(conn == NULL)||(conn->transactions == NULL)) return 0;
unsigned int i = 0;
for (i = 0; i < list_size(conn->transactions); i++) {

@ -135,6 +135,11 @@ htp_connp_t *htp_connp_create_copycfg(htp_cfg_t *cfg) {
if (connp == NULL) return NULL;
connp->cfg = htp_config_copy(cfg);
if (connp->cfg == NULL) {
htp_connp_destroy(connp);
return NULL;
}
connp->is_cfg_private = 1;
return connp;
@ -160,7 +165,8 @@ void htp_connp_destroy(htp_connp_t *connp) {
free(connp->in_header_line);
}
free(connp->in_line);
if (connp->in_line != NULL)
free(connp->in_line);
if (connp->out_header_line != NULL) {
if (connp->out_header_line->line != NULL) {
@ -170,12 +176,14 @@ void htp_connp_destroy(htp_connp_t *connp) {
free(connp->out_header_line);
}
free(connp->out_line);
if (connp->out_line != NULL)
free(connp->out_line);
// Destroy the configuration structure, but only
// if it is our private copy
if (connp->is_cfg_private) {
htp_config_destroy(connp->cfg);
if (connp->cfg != NULL)
htp_config_destroy(connp->cfg);
}
free(connp);
@ -188,15 +196,12 @@ void htp_connp_destroy(htp_connp_t *connp) {
* @param connp
*/
void htp_connp_destroy_all(htp_connp_t *connp) {
if (connp->conn == NULL) {
fprintf(stderr, "HTP: htp_connp_destroy_all was invoked, but conn is NULL\n");
return;
if (connp->conn != NULL) {
// Destroy connection
htp_conn_destroy(connp->conn);
connp->conn = NULL;
}
// Destroy connection
htp_conn_destroy(connp->conn);
connp->conn = NULL;
// Destroy everything else
htp_connp_destroy(connp);
}

@ -23,7 +23,7 @@
* @return Protocol version or PROTOCOL_UKNOWN.
*/
int htp_parse_protocol(bstr *protocol) {
if (bstr_len(protocol) == 8) {
if (protocol != NULL && bstr_len(protocol) == 8) {
char *ptr = bstr_ptr(protocol);
if ((ptr[0] == 'H') && (ptr[1] == 'T') && (ptr[2] == 'T') && (ptr[3] == 'P')
&& (ptr[4] == '/') && (ptr[6] == '.')) {

@ -266,7 +266,7 @@ int htp_connp_REQ_BODY_DETERMINE(htp_connp_t *connp) {
// Check for the Transfer-Encoding header, which
// would indicate a chunked request body
if (te != NULL) {
if (te != NULL && te->value != NULL) {
// Make sure it contains "chunked" only
if (bstr_cmpc(te->value, "chunked") != 0) {
// Invalid T-E header value
@ -299,7 +299,7 @@ int htp_connp_REQ_BODY_DETERMINE(htp_connp_t *connp) {
connp->in_tx->progress = TX_PROGRESS_REQ_BODY;
} else
// Next check for the presence of the Content-Length header
if (cl != NULL) {
if (cl != NULL && cl->value != NULL) {
// It seems that we have a request body.
connp->in_tx->request_transfer_coding = IDENTITY;

@ -52,6 +52,13 @@ int htp_process_request_header_apache_2_2(htp_connp_t *connp) {
for (i = connp->in_header_line_index; i < connp->in_header_line_counter; i++) {
htp_header_line_t *hl = list_get(connp->in_tx->request_header_lines, i);
if (hl == NULL) {
// Internal error
htp_log(connp, HTP_LOG_MARK, HTP_LOG_ERROR, 0,
"Process request header (Apache 2.2): Internal error");
free(h);
return HTP_ERROR;
}
len += bstr_len(hl->line);
}
@ -65,6 +72,14 @@ int htp_process_request_header_apache_2_2(htp_connp_t *connp) {
for (i = connp->in_header_line_index; i < connp->in_header_line_counter; i++) {
htp_header_line_t *hl = list_get(connp->in_tx->request_header_lines, i);
if (hl == NULL) {
// Internal error
htp_log(connp, HTP_LOG_MARK, HTP_LOG_ERROR, 0,
"Process request header (Apache 2.2): Internal error");
bstr_free(tempstr);
free(h);
return HTP_ERROR;
}
char *data = bstr_ptr(hl->line);
size_t len = bstr_len(hl->line);
htp_chomp((unsigned char *)data, &len);
@ -98,8 +113,10 @@ int htp_process_request_header_apache_2_2(htp_connp_t *connp) {
bstr_add_str_noex(h_existing->value, h->value);
// The header is no longer needed
free(h->name);
free(h->value);
if (h->name != NULL)
free(h->name);
if (h->value != NULL)
free(h->value);
free(h);
// Keep track of same-name headers
@ -276,6 +293,9 @@ int htp_parse_request_line_apache_2_2(htp_connp_t *connp) {
}
tx->request_uri = bstr_memdup((char *) data + start, pos - start);
if (tx->request_uri == NULL) {
return HTP_ERROR;
}
#ifdef HTP_DEBUG
fprint_raw_data(stderr, __FUNCTION__, (unsigned char *)bstr_ptr(tx->request_uri), bstr_len(tx->request_uri));

@ -86,27 +86,31 @@ void htp_tx_destroy(htp_tx_t *tx) {
// Destroy request_header_lines
htp_header_line_t *hl = NULL;
list_iterator_reset(tx->request_header_lines);
while ((hl = list_iterator_next(tx->request_header_lines)) != NULL) {
bstr_free(hl->line);
bstr_free(hl->terminators);
// No need to destroy hl->header because
// htp_header_line_t does not own it.
free(hl);
}
if (tx->request_header_lines != NULL) {
list_iterator_reset(tx->request_header_lines);
while ((hl = list_iterator_next(tx->request_header_lines)) != NULL) {
bstr_free(hl->line);
bstr_free(hl->terminators);
// No need to destroy hl->header because
// htp_header_line_t does not own it.
free(hl);
}
list_destroy(tx->request_header_lines);
list_destroy(tx->request_header_lines);
}
// Destroy request_headers
htp_header_t *h = NULL;
table_iterator_reset(tx->request_headers);
while (table_iterator_next(tx->request_headers, (void **) & h) != NULL) {
bstr_free(h->name);
bstr_free(h->value);
free(h);
}
if (tx->request_headers != NULL) {
table_iterator_reset(tx->request_headers);
while (table_iterator_next(tx->request_headers, (void **) & h) != NULL) {
bstr_free(h->name);
bstr_free(h->value);
free(h);
}
table_destroy(tx->request_headers);
table_destroy(tx->request_headers);
}
if (tx->request_headers_raw != NULL) {
bstr_free(tx->request_headers_raw);
@ -119,25 +123,29 @@ void htp_tx_destroy(htp_tx_t *tx) {
// Destroy response_header_lines
hl = NULL;
list_iterator_reset(tx->response_header_lines);
while ((hl = list_iterator_next(tx->response_header_lines)) != NULL) {
bstr_free(hl->line);
bstr_free(hl->terminators);
// No need to destroy hl->header because
// htp_header_line_t does not own it.
free(hl);
if (tx->response_header_lines != NULL) {
list_iterator_reset(tx->response_header_lines);
while ((hl = list_iterator_next(tx->response_header_lines)) != NULL) {
bstr_free(hl->line);
bstr_free(hl->terminators);
// No need to destroy hl->header because
// htp_header_line_t does not own it.
free(hl);
}
list_destroy(tx->response_header_lines);
}
list_destroy(tx->response_header_lines);
// Destroy response headers
h = NULL;
table_iterator_reset(tx->response_headers);
while (table_iterator_next(tx->response_headers, (void **) & h) != NULL) {
bstr_free(h->name);
bstr_free(h->value);
free(h);
if (tx->response_headers) {
table_iterator_reset(tx->response_headers);
while (table_iterator_next(tx->response_headers, (void **) & h) != NULL) {
bstr_free(h->name);
bstr_free(h->value);
free(h);
}
table_destroy(tx->response_headers);
}
table_destroy(tx->response_headers);
// Tell the connection to remove this transaction
// from the list

@ -147,6 +147,7 @@ int htp_is_space(int c) {
* @return Method number of M_UNKNOWN
*/
int htp_convert_method_to_number(bstr *method) {
if (method == NULL) return M_UNKNOWN;
// TODO Optimize using parallel matching, or something
if (bstr_cmpc(method, "GET") == 0) return M_GET;
if (bstr_cmpc(method, "PUT") == 0) return M_PUT;
@ -446,6 +447,8 @@ int htp_parse_authority(htp_connp_t *connp, bstr *authority, htp_uri_t **uri) {
* @return HTP_ERROR on memory allocation failure, HTP_OK otherwise
*/
int htp_parse_uri(bstr *input, htp_uri_t **uri) {
if (input == NULL)
return HTP_ERROR;
char *data = bstr_ptr(input);
size_t len = bstr_len(input);
size_t start, pos;
@ -920,6 +923,9 @@ int decode_u_encoding(htp_cfg_t *cfg, htp_tx_t *tx, unsigned char *data) {
* @param path
*/
int htp_decode_path_inplace(htp_cfg_t *cfg, htp_tx_t *tx, bstr *path) {
if (path == NULL)
return -1;
unsigned char *data = (unsigned char *) bstr_ptr(path);
if (data == NULL) {
return -1;
@ -1228,22 +1234,23 @@ int htp_normalize_parsed_uri(htp_connp_t *connp, htp_uri_t *incomplete, htp_uri_
if (incomplete->path != NULL) {
// Make a copy of the path, on which we can work on
normalized->path = bstr_strdup(incomplete->path);
if (normalized->path != NULL) {
// Decode URL-encoded (and %u-encoded) characters, as well as lowercase,
// compress separators and convert backslashes.
htp_decode_path_inplace(connp->cfg, connp->in_tx, normalized->path);
// Handle UTF-8 in path
if (connp->cfg->path_convert_utf8) {
// Decode Unicode characters into a single-byte stream, using best-fit mapping
htp_utf8_decode_path_inplace(connp->cfg, connp->in_tx, normalized->path);
} else {
// Only validate path as a UTF-8 stream
htp_utf8_validate_path(connp->in_tx, normalized->path);
}
// Decode URL-encoded (and %u-encoded) characters, as well as lowercase,
// compress separators and convert backslashes.
htp_decode_path_inplace(connp->cfg, connp->in_tx, normalized->path);
// Handle UTF-8 in path
if (connp->cfg->path_convert_utf8) {
// Decode Unicode characters into a single-byte stream, using best-fit mapping
htp_utf8_decode_path_inplace(connp->cfg, connp->in_tx, normalized->path);
} else {
// Only validate path as a UTF-8 stream
htp_utf8_validate_path(connp->in_tx, normalized->path);
// RFC normalization
htp_normalize_uri_path_inplace(normalized->path);
}
// RFC normalization
htp_normalize_uri_path_inplace(normalized->path);
}
// Query

Loading…
Cancel
Save