On Mon, Feb 10, 2020 at 11:41:15AM -0500, Konstantin Ryabitsev wrote: > This mostly becomes a problem if we try to build any kind of patch > indexing/retrieval systems that rely on patch-id to identify patches. > While this is not a high-impact problem by any means, it's not a > theoretical concern: git-format-patch includes functionality to provide > patch dependencies via prerequisite-patch-id trailers [1]. An automated > system attempting to auto-fetch dependencies can potentially retrieve > and apply the malicious version of the patch. > > I'm not sure what the solution here is, since changing git-patch-id to > not discard whitespace is obviously going to defeat its entire purpose > of "not ever changing". I mostly wanted to share my findings in case > someone has thoughts on how to best approach this. Can't you already have malicious patch-id collisions without the whitespace thing? The patch-id also throws away line numbers, so a patch adding "return 0" in an innocent location could have the same patch-id as one adding it somewhere more dangerous. It's just a question of the context, but there's often enough boilerplate for two functions to look similar. This is occasionally a problem for actual accidental collisions (in patch-ids, but also when merging). I can imagine it's probably not that hard for a determined attacker to make such a case intentionally. -Peff
