Am 01.03.25 um 07:43 schrieb H Z: > Hi, I am a static analysis tool developer, and I have found a > potential null pointer dereference bug in src/read-cache.h 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. > > Below is the execution sequence of the program that may produce the bug. > First, in the file diff-lib.c, the function oneway_diff assigns tree > to NULL on line 537. Context: const struct cache_entry *idx = src[0]; const struct cache_entry *tree = src[1]; /* * Unpack-trees generates a DF/conflict entry if * there was a directory in the index and a tree * in the tree. From a diff standpoint, that's a * delete of the tree and a create of the file. */ if (tree == o->df_conflict_entry) tree = NULL; What's a DF conflict? A directory (D) in the index and a file (F) in the tree for the same path, or vice versa. Except that in the object database directories are represented by "trees" and files by "blobs", but that's not important here. Not sure why "a tree in the tree" would conflict with a directory, though, as the comment claims. Probably a typo, and it should read "a blob in the tree" instead. > Second, if on line 540, idx ? idx : tree conditional judgement is > false, tree, which is NULL, is passed as the 2nd argument to the > function ce_path_match. This should never happen, as we don't call a situation where one side is absent (i.e. an added or removed file or directory) a DF conflict, i.e. (tree == o->df_conflict_entry) and (idx == NULL) should never be true at the same time. Now it would be very interesting if that assumption could actually be violated by callers. That's a bit hard to prove across the indirect call by function pointer, I guess. > Third, in file read-cache.h, the parameter ce of function > ce_path_match is assigned to NULL. and ce is dereferenced on line 41, > resulting in a null pointer dereference vulnerability. We could add a defensive check, but is it worth it? If we get it wrong we'd get a segfault, at least on our main target platforms. René
