Am 01.03.25 um 08:33 schrieb H Z: > Hi, I am a static analysis tool developer, and I have found a > potential null pointer dereference bug in commit.c and would like to > report it to the maintainers. This vulnerability has the potential to > cause unexpected application behavior, crashes. Can you please help me > check it? Thank you for your effort and patience! > > Below is the execution sequence of the program that may produce the > null pointer dereference bug. > > First, in the file commit.c, the function pop_commit may assign item > to NULL at line 806 if the conditional judgement is false. True. > Second, in file commit.c, function pop_most_recent_commit calls > function pop_commit at line 748, which may cause variable ret to be > assigned NULL. Technically true, but not quite. I understand that this is effectively a trap for analysis tools, unfortunately. pop_commit() handles empty commit_lists, while pop_most_recent_commit() doesn't, despite its similar name. All three callers of the latter make sure to pass only pointers to non-NULL commit_lists, so we're actually safe. > Finally, ret is dereferenced on line 749, leading to a null pointer > dereference vulnerability. > > However, in the file merge-ort.c, the function merge_ort_internal > calls the function pop_commit on line 5176, and then makes a judgement > on whether the return value of pop_commit is NULL or not on line 5177, > which suggests that it is indeed possible for pop_commit to return > NULL. True. Most callers of pop_commit() check for NULL, so that function could stop checking as well, to slightly increase efficiency and simplicity. We would "just" have to audit every caller and make sure they are all ready for that. The only ones that need modifying seem to be in pack-bitmap-write.c and revision.c. Is it worth it? Not sure. René
