commit message parsing error in rebase

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

The bug I'm to report is objectively a bug; nevertheless, let
me use a fictive scenario to illustrate it.

Say we have a project where we fix a vulnerability in the
server component. In the commit message of the fix, we
give a PoC malicious client that can be used to test if
server is vulnerable. The PoC is specified by embedding
a diff against normal client in the commit message.

(Let's not discuss how good or bad it is to give PoCs in
commit messages.)

Now if you rebase this commit, git-rebase will think the
PoC diff in the commit message is part of the commit diff,
and will apply it. So the client will became malicious
upon the rebase.

Nb. cherry-pick does not make this mistake (what took
me by surprise as I thought rebase and cherry-pick have a
common backend).

The following script creates such a scenario:

####################################################
echo 'def client(conn):
  print >> conn, "hello"' > client.py
echo 'def server(conn):
  print conn.read()[4]' > server.py
git add .
git commit -m first
echo 'def server(conn):
  msg = conn.read()
  if len(msg) >= 5:
    print msg[4]' > server.py
git commit -am 'server: check msg size

Make sure client cannot crash server.

Q/A: check with PoC malicious client
by applying patch:

diff --git a/client.py b/client.py
--- a/client.py
+++ b/client.py
@@ -1,2 +1,2 @@
 def client(conn):
-  print >> conn, "hello"
+  print >> conn, "hell"'
git checkout --detach HEAD^
echo 'def load_balancer():
  balance_load()' > load_balancer.py
git add load_balancer.py
git commit -m 'added load balancer prototype'
git tag LOAD_BALANCER_PROTO
git branch bleeding-edge master
git rebase LOAD_BALANCER_PROTO bleeding-edge
git checkout LOAD_BALANCER_PROTO
git cherry-pick master
echo
echo '###### RESULT #######'
echo '### '`git --version`
echo
echo '## topolgy'
git log --graph --pretty=oneline --all --decorate
echo
echo '## original commit'
git show --stat --decorate master
echo
echo '## rebased commit'
git show --stat --decorate bleeding-edge
echo
echo '## cherry-picked commit'
git show --stat --decorate HEAD
####################################################

and it has an output like this:

###### RESULT #######
### git version 1.7.10.1

## topolgy
* f5100d373242554f71db47592d8214c9e89d854a (bleeding-edge) server:
check msg size
| * c8fa20bd901d3bc0e745d976151d3b34d91f077d (HEAD) server: check msg size
|/
* 0cb17691eb9a9774af0fc991bf34da89fd413325 (tag: LOAD_BALANCER_PROTO)
added load balancer prototype
| * 3628281d81d275f431bdc0da36fe7c7d66240175 (master) server: check msg size
|/
* 80d8113aa7f6fa46245408ddbc846d9c1e796373 first

## original commit
commit 3628281d81d275f431bdc0da36fe7c7d66240175 (master)
Author: Csaba Henk <>
Date:   Fri May 4 23:56:51 2012 +0530

    server: check msg size

    Make sure client cannot crash server.

    Q/A: check with PoC malicious client
    by applying patch:

    diff --git a/client.py b/client.py
    --- a/client.py
    +++ b/client.py
    @@ -1,2 +1,2 @@
     def client(conn):
    -  print >> conn, "hello"
    +  print >> conn, "hell"

 server.py |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

## rebased commit
commit f5100d373242554f71db47592d8214c9e89d854a (bleeding-edge)
Author: Csaba Henk <>
Date:   Fri May 4 23:56:51 2012 +0530

    server: check msg size

    Make sure client cannot crash server.

    Q/A: check with PoC malicious client
    by applying patch:

    diff --git a/client.py b/client.py
    --- a/client.py
    +++ b/client.py
    @@ -1,2 +1,2 @@
     def client(conn):
    -  print >> conn, "hello"
    +  print >> conn, "hell"

 client.py |    2 +-
 server.py |    4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

## cherry-picked commit
commit c8fa20bd901d3bc0e745d976151d3b34d91f077d (HEAD)
Author: Csaba Henk <>
Date:   Fri May 4 23:56:51 2012 +0530

    server: check msg size

    Make sure client cannot crash server.

    Q/A: check with PoC malicious client
    by applying patch:

    diff --git a/client.py b/client.py
    --- a/client.py
    +++ b/client.py
    @@ -1,2 +1,2 @@
     def client(conn):
    -  print >> conn, "hello"
    +  print >> conn, "hell"

 server.py |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


Csaba
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]