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