From 0a258bd60afc321dc3d1944c1ef838c8446a67cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20R=C3=BChsen?= Date: Mon, 14 Sep 2026 11:53:10 +0200 Subject: [PATCH] Make use of clang's ownership_takes attribute This allows scan-build to find a use-after-free introduced by commit 581b53633159200125bcdee697762c62449b1067 (not released). * bootstrap.conf: Add attribute gnulib module. * src/wget.h: Define ATTRIBUTE_OWNERSHIP_TAKES. * src/html-url.h: Make use of ATTRIBUTE_OWNERSHIP_TAKES. * src/http-ntlm.h: Likewise. * src/iri.h: Likewise. * src/ptimer.h: Likewise. * src/url.c: New function url_new() as a wraper to xnew0() to suppress a -Wmismatched-dealloc warning. * src/url.h: Make use of ATTRIBUTE_OWNERSHIP_TAKES. * src/utils.h: Likewise. --- bootstrap.conf | 1 + src/html-url.h | 13 +++++++++---- src/http-ntlm.h | 1 + src/iri.h | 12 ++++++++++-- src/ptimer.h | 4 +++- src/url.c | 10 +++++++++- src/url.h | 21 ++++++++++++++++++++- src/utils.h | 21 +++++++++++++++++++-- src/wget.h | 9 +++++++++ 9 files changed, 81 insertions(+), 11 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index b02c6daf..46bb1f56 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -29,6 +29,7 @@ checkout_only_file= gnulib_modules=" accept announce-gen +attribute base32 bind c-ctype diff --git a/src/html-url.h b/src/html-url.h index 552ea9a9..7f49f3b6 100644 --- a/src/html-url.h +++ b/src/html-url.h @@ -48,11 +48,16 @@ struct map_context { struct urlpos *head; /* List of URLs that is being built. */ }; -struct urlpos *get_urls_file (const char *, bool *); -struct urlpos *get_urls_html (const char *, const char *, bool *, struct iri *); -struct urlpos *get_urls_html_fm (const char *, const struct file_memory *, const char *, bool *, struct iri *); -struct urlpos *append_url (const char *, int, int, struct map_context *); +ATTRIBUTE_OWNERSHIP_TAKES(malloc, 1) void free_urlpos (struct urlpos *); +ATTRIBUTE_DEALLOC(free_urlpos, 1) +struct urlpos *get_urls_file (const char *, bool *); +ATTRIBUTE_DEALLOC(free_urlpos, 1) +struct urlpos *get_urls_html (const char *, const char *, bool *, struct iri *); +ATTRIBUTE_DEALLOC(free_urlpos, 1) +struct urlpos *get_urls_html_fm (const char *, const struct file_memory *, const char *, bool *, struct iri *); +ATTRIBUTE_DEALLOC(free_urlpos, 1) +struct urlpos *append_url (const char *, int, int, struct map_context *); void cleanup_html_url (void); #endif /* HTML_URL_H */ diff --git a/src/http-ntlm.h b/src/http-ntlm.h index 5b47bac0..3faa91ce 100644 --- a/src/http-ntlm.h +++ b/src/http-ntlm.h @@ -49,5 +49,6 @@ struct ntlmdata { bool ntlm_input (struct ntlmdata *, const char *); /* this is for creating ntlm header output */ +ATTRIBUTE_MALLOC ATTRIBUTE_DEALLOC_FREE char *ntlm_output (struct ntlmdata *, const char *, const char *, bool *); #endif diff --git a/src/iri.h b/src/iri.h index 5d3961c1..26af701d 100644 --- a/src/iri.h +++ b/src/iri.h @@ -31,6 +31,8 @@ as that of the covered work. */ #ifndef IRI_H #define IRI_H +#include "wget.h" + struct iri { char *uri_encoding; /* Encoding of the uri to fetch */ char *content_encoding; /* Encoding of links inside the fetched file */ @@ -40,16 +42,22 @@ struct iri { #ifdef ENABLE_IRI +ATTRIBUTE_MALLOC ATTRIBUTE_DEALLOC_FREE char *parse_charset (const char *str); const char *find_locale (void); bool check_encoding_name (const char *encoding); const char *locale_to_utf8 (const char *str); +ATTRIBUTE_MALLOC ATTRIBUTE_DEALLOC_FREE char *idn_encode (const struct iri *i, const char *host); +ATTRIBUTE_MALLOC ATTRIBUTE_DEALLOC_FREE char *idn_decode (const char *host); bool remote_to_utf8 (const struct iri *i, const char *str, char **newstr); -struct iri *iri_new (void); -struct iri *iri_dup (const struct iri *); +ATTRIBUTE_OWNERSHIP_TAKES(malloc, 1) void iri_free (struct iri *i); +ATTRIBUTE_DEALLOC(iri_free, 1) +struct iri *iri_new (void); +ATTRIBUTE_DEALLOC(iri_free, 1) +struct iri *iri_dup (const struct iri *); void set_uri_encoding (struct iri *i, const char *charset, bool force); void set_content_encoding (struct iri *i, const char *charset); diff --git a/src/ptimer.h b/src/ptimer.h index a9e30036..b5756fbd 100644 --- a/src/ptimer.h +++ b/src/ptimer.h @@ -34,8 +34,10 @@ as that of the covered work. */ struct ptimer; /* forward declaration; all struct members are private */ -struct ptimer *ptimer_new (void); +ATTRIBUTE_OWNERSHIP_TAKES(malloc, 1) void ptimer_destroy (struct ptimer *); +ATTRIBUTE_MALLOC ATTRIBUTE_DEALLOC(ptimer_destroy, 1) +struct ptimer *ptimer_new (void); void ptimer_reset (struct ptimer *); double ptimer_measure (struct ptimer *); diff --git a/src/url.c b/src/url.c index f816e1df..50b63a82 100644 --- a/src/url.c +++ b/src/url.c @@ -714,6 +714,14 @@ static const char *parse_errors[] = { [PE_INVALID_IPV6_ADDRESS] = N_("Invalid IPv6 numeric address") }; +/* This wrapper suppresses a -Wmismatched-dealloc, + which is triggered by calling url_free() inside url_parse(). */ +static struct url * +url_new(void) +{ + return xnew0(struct url); +} + /* Parse a URL. Return a new struct url if successful, NULL on error. In case of @@ -929,7 +937,7 @@ url_parse (const char *url, int *error, struct iri *iri, bool percent_encode) } } - u = xnew0 (struct url); + u = url_new (); u->scheme = scheme; u->host = strdupdelim (host_b, host_e); u->port = port; diff --git a/src/url.h b/src/url.h index ba220ab1..351c4b43 100644 --- a/src/url.h +++ b/src/url.h @@ -47,6 +47,8 @@ as that of the covered work. */ * file descriptor. */ #define CHOMP_BUFFER 19 +#include "wget.h" + /* The flags that allow clobbering the file (opening with "wb"). Defined here to avoid repetition later. #### This will require rework. */ @@ -102,17 +104,28 @@ struct url /* Function declarations */ +ATTRIBUTE_MALLOC ATTRIBUTE_DEALLOC_FREE char *url_escape (const char *); + +ATTRIBUTE_MALLOC ATTRIBUTE_DEALLOC_FREE char *url_escape_unsafe_and_reserved (const char *); + void url_unescape (char *); void url_unescape_except_reserved (char *); +ATTRIBUTE_OWNERSHIP_TAKES(malloc, 1) +void url_free (struct url *); + +ATTRIBUTE_DEALLOC(url_free, 1) struct url *url_parse (const char *, int *, struct iri *iri, bool percent_encode); + const char *url_error (int); + +ATTRIBUTE_MALLOC ATTRIBUTE_DEALLOC_FREE char *url_full_path (const struct url *); + void url_set_dir (struct url *, const char *); void url_set_file (struct url *, const char *); -void url_free (struct url *); enum url_scheme url_scheme (const char *); bool url_has_scheme (const char *); @@ -121,14 +134,20 @@ int scheme_default_port (enum url_scheme); void scheme_disable (enum url_scheme); const char *scheme_leading_string (enum url_scheme); +ATTRIBUTE_MALLOC ATTRIBUTE_DEALLOC_FREE char *url_string (const struct url *, enum url_auth_mode); + +ATTRIBUTE_MALLOC ATTRIBUTE_DEALLOC_FREE char *url_file_name (const struct url *, char *); +ATTRIBUTE_MALLOC ATTRIBUTE_DEALLOC_FREE char *uri_merge (const char *, const char *); int mkalldirs (const char *); +ATTRIBUTE_MALLOC ATTRIBUTE_DEALLOC_FREE char *maybe_prepend_scheme (const char *); + bool schemes_are_similar_p (enum url_scheme a, enum url_scheme b); bool are_urls_equal (const char *u1, const char *u2); diff --git a/src/utils.h b/src/utils.h index 0236da66..af6cc6aa 100644 --- a/src/utils.h +++ b/src/utils.h @@ -66,14 +66,20 @@ struct file_memory { char *time_str (time_t); char *datetime_str (time_t); +ATTRIBUTE_MALLOC ATTRIBUTE_DEALLOC_FREE char *xstrdup_lower (const char *); +ATTRIBUTE_MALLOC ATTRIBUTE_DEALLOC_FREE char *strdupdelim (const char *, const char *); + char **sepstring (const char *); bool subdir_p (const char *, const char *); bool fork_to_background (void); +ATTRIBUTE_MALLOC ATTRIBUTE_DEALLOC_FREE char *aprintf (const char *, ...) GCC_FORMAT_ATTR (1, 2); + +ATTRIBUTE_MALLOC ATTRIBUTE_DEALLOC_FREE char *concat_strings (const char *, ...); typedef struct file_stat_s { @@ -89,12 +95,17 @@ bool file_non_directory_p (const char *); wgint file_size (const char *); int make_directory (const char *); char *unique_name_passthrough (const char *); + +ATTRIBUTE_MALLOC ATTRIBUTE_DEALLOC_FREE char *unique_name (const char *); + FILE *unique_create (const char *, bool, char **); FILE *fopen_excl (const char *, int); FILE *fopen_nofollow (const char *, const char *); FILE *fopen_stat (const char *, const char *, file_stats_t *); int open_stat (const char *, int, mode_t, file_stats_t *); + +ATTRIBUTE_MALLOC ATTRIBUTE_DEALLOC_FREE char *file_merge (const char *, const char *); int fnmatch_nocase (const char *, const char *, int); @@ -107,12 +118,18 @@ bool has_wildcards_p (const char *); bool has_html_suffix_p (const char *); -struct file_memory *wget_read_from_file (const char *, bool *); -struct file_memory *wget_read_file (const char *); +ATTRIBUTE_OWNERSHIP_TAKES(malloc, 1) void wget_read_file_free (struct file_memory *); +ATTRIBUTE_DEALLOC(wget_read_file_free, 1) +struct file_memory *wget_read_from_file (const char *, bool *); +ATTRIBUTE_DEALLOC(wget_read_file_free, 1) + struct file_memory *wget_read_file (const char *); +ATTRIBUTE_OWNERSHIP_TAKES(malloc, 1) void free_vec (char **); +ATTRIBUTE_DEALLOC(free_vec, 1) char **merge_vecs (char **, char **); +ATTRIBUTE_DEALLOC(free_vec, 1) char **vec_append (char **, const char *); void string_set_add (struct hash_table *, const char *); diff --git a/src/wget.h b/src/wget.h index 92c3934e..d5e12a7d 100644 --- a/src/wget.h +++ b/src/wget.h @@ -112,6 +112,15 @@ as that of the covered work. */ # define UNLIKELY(exp) (exp) #endif +#include "attribute.h" + +#if defined(__clang_analyzer__) && defined(__clang__) +# define ATTRIBUTE_OWNERSHIP_TAKES(type, arg) \ + __attribute__((ownership_takes(type, arg))) +#else +# define ATTRIBUTE_OWNERSHIP_TAKES(type, arg) +#endif + /* Execute the following statement if debugging is both enabled at compile-time and requested at run-time; a no-op otherwise. */