Re: [PATCH 01/23] builtin/help: fix dangling reference to `html_path`
- From
Justin Tobler <jltobler@gmail.com>
- Date
- Sep 16, 2024, 16:24 UTC
- Message-ID
- <guzjej2ghio2bxxt6epofcnl57nqaoagm65byox4iwhvkim2oy@qcuyzzsw5que>
- In-Reply-To
- <e3bed973afacaec801cff1e77aeea6050cb34f57.1726484308.git.ps@pks.im>
On 24/09/16 01:45PM, Patrick Steinhardt wrote:
Show 29 quoted lines
> In `get_html_page_path()` we may end up assigning the return value of
> `system_path()` to the global `html_path` variable. But as we also
> assign the returned value to `to_free`, we will deallocate its memory
> upon returning from the function. Consequently, `html_path` will now
> point to deallocated memory.
>
> Fix this issue by instead assigning the value to a separate local
> variable.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> builtin/help.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/builtin/help.c b/builtin/help.c
> index dc1fbe2b986..282ea5721fa 100644
> --- a/builtin/help.c
> +++ b/builtin/help.c
> @@ -513,23 +513,24 @@ static void show_info_page(const char *page)
> static void get_html_page_path(struct strbuf *page_path, const char *page)
> {
> struct stat st;
> + const char *path = html_path;
> char *to_free = NULL;
>
> - if (!html_path)
> - html_path = to_free = system_path(GIT_HTML_PATH);
> + if (!path)
> + path = to_free = system_path(GIT_HTML_PATH);Previously, `html_path` was being assigned the return value of `system_path()` and consequently escaping this scope even though the value was being freed. There is no reason to modify the global value to begin with so we instead assign the value to a local variable. Makes sense.
Show 24 quoted lines
>
> /*
> * Check that the page we're looking for exists.
> */
> - if (!strstr(html_path, "://")) {
> - if (stat(mkpath("%s/%s.html", html_path, page), &st)
> + if (!strstr(path, "://")) {
> + if (stat(mkpath("%s/%s.html", path, page), &st)
> || !S_ISREG(st.st_mode))
> die("'%s/%s.html': documentation file not found.",
> - html_path, page);
> + path, page);
> }
>
> strbuf_init(page_path, 0);
> - strbuf_addf(page_path, "%s/%s.html", html_path, page);
> + strbuf_addf(page_path, "%s/%s.html", path, page);
> free(to_free);
> }
>
> --
> 2.46.0.551.gc5ee8f2d1c.dirty
>
>