Re: git log -p unexpected behaviour - security risk?

From: John Tapsell <johnflux@gmail.com>
Date: Apr 21, 2013, 13:46 UTC
Message-ID: <CAHQ6N+rXE42NOyQPfLiDN8jYfL8w06hEE5MFLeFNxMR4ORD0aw@mail.gmail.com>
In-Reply-To: <20130421102150.GJ10429@elie.Belkin>

On 21 April 2013 11:21, Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 7 quoted lines

> John Tapsell wrote:
>
>> I'm concerned that noone is taking this security risk seriously.
>
> If anyone relies on "git log -p" or "git log -p --cc" output to make
> sure that the untrusted code they use doesn't introduce unwanted
> behavior, they are making a serious mistake.

Which is exactly my problem.

Go and ask the average person using git this very question, and I bet you the vast majority will not know about -cc etc.

You can't just push all the blame on the user for bad defaults. Hiding code changes is a bad default.

> A merge can completely
> undo important changes made in a side branch and "-c" and "--cc" will
> not show it.

Wait, what?  This is getting even worse then!  Can you expand on this please?

And then how do I show all of these important changes with a git log -p ? Or is it impossible to get a sane output?

>  The lack of "-c" cannot be a security issue here,
> because in normal life adding "-c" isn't a secure deployment strategy.

So, is it impossible to make git log -p a "secure deployment strategy" ?

> That's why if you want to review the code you are pulling in as a
> whole, it is worthwhile to do
>
>         git diff HEAD...FETCH_HEAD

Which basically means that you're asking the review the same code twice. Once that way, and once using git log -p (to check for the exact reason that you said).

>  Unfortunately that doesn't protect you from
> maliciously written commits that will be encountered when bisecting.
> At some point you have to be able to trust people.

Seriously? Your reasoning for awful defaults is that you should just trust people?

This is getting worse and worse!

John