Unsound lifetime bound in Remote::get_refspec

[kimjy0126]

Remote::get_refspec returns a RefSpec with lifetime 'repo, not the same lifetime as &self. This allows Refspec to outlive Remote (self).

git2-rs/src/remote.rs

Lines 301 to 306 in a00922b

	pub fn get_refspec(&self, i: usize) -> Option<Refspec<'repo>> {
	unsafe {
	let ptr = raw::git_remote_get_refspec(&*self.raw, i as libc::size_t);
	Binding::from_raw_opt(ptr)
	}
	}

Reproduction

#[forbid(unsafe_code)]

use git2::Repository;

fn main() {
    let repo = Repository::init("temp_dir").unwrap();
    repo.remote("origin", "https://aaa.com/bbb.git").unwrap();
    
    let refspec = {
        let remote = repo.find_remote("origin").unwrap();
        remote.get_refspec(0)
    };
    // remote goes out of scope
    // but refspec's lifetime is not bounded to remote    
    
    // removing temp_dir, not necessary to trigger UAF
    let _ = std::fs::remove_dir_all("temp_dir");
    
    // triggers UAF
    let _ = refspec.unwrap().str();
}

You can get ASan UAF report by running following command. git2 latest release version (0.21.0) and latest commit in main branch (a00922b) print the same ASan report.

CFLAGS=-fsanitize=address RUSTFLAGS="-Zsanitizer=address" cargo +nightly run --target x86_64-unknown-linux-gnu

I reported this issue to security team first, and they said it's ok to make this issue publicly.

[DanielEScherzer]

let _ = std::fs::remote_dir_all("temp_dir"); should probably be remove_dir_all
I confirmed that ASAN does indeed complain locally, working on the patch now

[DanielEScherzer]

@rustbot label +bug

[kimjy0126]

let _ = std::fs::remote_dir_all("temp_dir"); should probably be remove_dir_all I confirmed that ASAN does indeed complain locally, working on the patch now

oh right, I fixed typo there, thanks