Skip to content

file-manager: fix double-free and relative path crash for submodule gitdir pointers - #1892

Open
Inspirati wants to merge 1 commit into
mate-desktop:masterfrom
Inspirati:fix/gitdir-relative-path-double-free
Open

Inspirati wants to merge 1 commit into
mate-desktop:masterfrom
Inspirati:fix/gitdir-relative-path-double-free

Conversation

@Inspirati

Copy link
Copy Markdown
Contributor

Problem

When Caja enumerates a directory whose subdirectories contain a .git file
(rather than a .git directory — as used by git submodules and linked
worktrees), get_git_branch() crashes intermittently via heap corruption.

The root cause is a double-free:

  1. Inside get_git_branch(), when .git is a regular file the code calls
    g_free(git_path) on its own const char * parameter (to free the
    caller's allocation before reassigning the local variable).
  2. The caller then calls g_free(git_path) again on the same pointer after
    get_git_branch() returns.

The resulting heap corruption is non-deterministic — it manifests as a crash
elsewhere in the process, not at the site of the double-free, which is why a
desktop code review found no obvious NULL-dereference path in the feature code.

A secondary issue: g_build_filename() does not canonicalise ../ components
in relative gitdir: paths. If the resolved path does not exist on disk
(e.g. an uninitialised submodule), realpath() returns NULL; without a guard
this would be passed into subsequent GLib calls.

Fix

  • Introduce a resolved_gitdir local variable to hold the heap-allocated
    resolved path. The git_path parameter (which belongs to the caller) is
    never freed inside the function.
  • Use realpath() to canonicalise relative gitdir: paths containing ../
    components. The result is copied into GLib-managed memory with g_strdup()
    and the malloc()-allocated realpath() buffer is freed with free().
  • Guard against a NULL return from realpath() — when the target path is
    absent on disk, effective_gitdir is set to NULL and the subsequent
    g_file_test() call is skipped cleanly.
  • Add #include <stdlib.h> for realpath() and free().

Crash reproduction

Navigate to a parent directory in Caja icon view that contains a subdirectory
with a .git file whose gitdir: line holds a relative path
(e.g. gitdir: ../../../../../.git/modules/...). With the Display Git Branch
preference enabled, Caja crashes on directory enumeration. After this fix,
enumeration completes without crashing.

… pointers

When a .git file contains a relative gitdir: path (as used by submodules
and linked worktrees), get_git_branch() was calling g_free() on its own
const char* parameter, then the caller would free the same pointer again.
This double-free causes heap corruption that manifests as a crash when
enumerating a parent directory containing such a submodule.

Fix by introducing a separate resolved_gitdir variable to hold the
heap-allocated resolved path, leaving the parameter untouched. Use
realpath() to canonicalise ../ components in relative paths, and guard
against a NULL return when the target path does not exist on disk.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@lukefromdc lukefromdc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking closely at this due to its AI origin. I don't see any obvious AI screwups here, but I don't know this part of the code well enough to follow the flow of it from just what we see here

@vkareh

vkareh commented Jun 4, 2026

Copy link
Copy Markdown
Member

The commit in this PR is included in #1893, which I reviewed earlier today. I think we should leave this here open for now until we decide whether we use one or the other (the fix is valid in either case, with some small changes)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants