NULL pointer dereference in git_submodule_add_setup on a repository without a working directory #7461

First of all, thank you so much for maintaining libgit2; while applying our testing tool across multiple Rust crates during its evaluation, we encountered an issue in libgit2 (reached through the Rust binding git2) that I would like to bring to your attention.

git_submodule_add_setup() dereferences a NULL pointer when the repository has no working directory, for example one created with git_repository_wrap_odb().

I notice the following code may have a problem:

Describe the bug

/* validate and normalize path */

if (git__prefixcmp(path, git_repository_workdir(repo)) == 0)
    path += strlen(git_repository_workdir(repo));

git_repository_workdir() returns NULL when the repository has no working directory. git__prefixcmp() then reads *prefix from that NULL pointer (src/util/util.c#L246). The function checks repo, url and path with GIT_ASSERT_ARG, but not the working directory.

A repository made by git_repository_wrap_odb() is not reported as bare by git_repository_is_bare(), so git_repository__ensure_not_bare() would not catch this case either. The working directory itself has to be checked.

Tested on libgit2 1.9.7, the latest release (as bundled by libgit2-sys 0.18.8+1.9.7, used by git2 0.21.0). Current main (42a89d7) has the same code.

To Reproduce

1. Create a repository without a working directory with git_odb_new + git_repository_wrap_odb.
2. Call git_submodule_add_setup(&sm, repo, "url", "p", 0).

Through the safe Rust binding git2 0.21.0:

fn main() {
    // a repository with an object database only, no working directory
    let repo = git2::Repository::from_odb(git2::Odb::new().unwrap()).unwrap();

    let _ = repo.submodule("url", std::path::Path::new("p"), false);
}

Expected behavior

The call should return an error, not crash the process.

output (AddressSanitizer, libgit2 built with -fsanitize=address):

==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000
==The signal is caused by a READ memory access.
    #0 git__prefixcmp
    #1 git_submodule_add_setup
    #2 git2::repo::Repository::submodule

Without AddressSanitizer the process is killed by SIGSEGV.

Suggested fix

Return an error when there is no working directory (src/libgit2/submodule.c, git_submodule_add_setup):

GIT_ASSERT_ARG(repo);
GIT_ASSERT_ARG(url);
GIT_ASSERT_ARG(path);

/* a submodule path is relative to the working directory, which a bare or
 * odb-only repository does not have */
if (git_repository_workdir(repo) == NULL) {
    git_error_set(GIT_ERROR_SUBMODULE,
        "cannot add a submodule to a repository without a working directory");
    return GIT_EBAREREPO;
}

I applied this to the libgit2 1.9.7 sources bundled in libgit2-sys. The reproducer then gets GIT_EBAREREPO with no crash.

Thank you for your time.