{"thread":{"id":"30423","subject":"commit message parsing error in rebase","startedAt":"2012-05-04T18:35:35Z","lastAt":"2012-05-04T19:39:01Z","messageCount":4,"participants":["Csaba Henk","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"190777","messageId":"CAA-jfp=F7ije9nFJio5gnmNGptDR1ZTBbCi-aOv7JokqrXBgYA@mail.gmail.com","threadId":"30423","inReplyTo":null,"subject":"commit message parsing error in rebase","fromName":"Csaba Henk","fromEmail":"csaba@lowlife.hu","sentAt":"2012-05-04T18:35:35Z","receivedAt":"2012-05-04T18:35:35Z","isPatch":false,"sender":{"key":"csaba@gluster.com","avatar":"https://avatars.githubusercontent.com/u/43937?v=4"},"body":"Hi,\n\nThe bug I'm to report is objectively a bug; nevertheless, let\nme use a fictive scenario to illustrate it.\n\nSay we have a project where we fix a vulnerability in the\nserver component. In the commit message of the fix, we\ngive a PoC malicious client that can be used to test if\nserver is vulnerable. The PoC is specified by embedding\na diff against normal client in the commit message.\n\n(Let's not discuss how good or bad it is to give PoCs in\ncommit messages.)\n\nNow if you rebase this commit, git-rebase will think the\nPoC diff in the commit message is part of the commit diff,\nand will apply it. So the client will became malicious\nupon the rebase.\n\nNb. cherry-pick does not make this mistake (what took\nme by surprise as I thought rebase and cherry-pick have a\ncommon backend).\n\nThe following script creates such a scenario:\n\n####################################################\necho 'def client(conn):\n  print >> conn, \"hello\"' > client.py\necho 'def server(conn):\n  print conn.read()[4]' > server.py\ngit add .\ngit commit -m first\necho 'def server(conn):\n  msg = conn.read()\n  if len(msg) >= 5:\n    print msg[4]' > server.py\ngit commit -am 'server: check msg size\n\nMake sure client cannot crash server.\n\nQ/A: check with PoC malicious client\nby applying patch:\n\ndiff --git a/client.py b/client.py\n--- a/client.py\n+++ b/client.py\n@@ -1,2 +1,2 @@\n def client(conn):\n-  print >> conn, \"hello\"\n+  print >> conn, \"hell\"'\ngit checkout --detach HEAD^\necho 'def load_balancer():\n  balance_load()' > load_balancer.py\ngit add load_balancer.py\ngit commit -m 'added load balancer prototype'\ngit tag LOAD_BALANCER_PROTO\ngit branch bleeding-edge master\ngit rebase LOAD_BALANCER_PROTO bleeding-edge\ngit checkout LOAD_BALANCER_PROTO\ngit cherry-pick master\necho\necho '###### RESULT #######'\necho '### '`git --version`\necho\necho '## topolgy'\ngit log --graph --pretty=oneline --all --decorate\necho\necho '## original commit'\ngit show --stat --decorate master\necho\necho '## rebased commit'\ngit show --stat --decorate bleeding-edge\necho\necho '## cherry-picked commit'\ngit show --stat --decorate HEAD\n####################################################\n\nand it has an output like this:\n\n###### RESULT #######\n### git version 1.7.10.1\n\n## topolgy\n* f5100d373242554f71db47592d8214c9e89d854a (bleeding-edge) server:\ncheck msg size\n| * c8fa20bd901d3bc0e745d976151d3b34d91f077d (HEAD) server: check msg size\n|/\n* 0cb17691eb9a9774af0fc991bf34da89fd413325 (tag: LOAD_BALANCER_PROTO)\nadded load balancer prototype\n| * 3628281d81d275f431bdc0da36fe7c7d66240175 (master) server: check msg size\n|/\n* 80d8113aa7f6fa46245408ddbc846d9c1e796373 first\n\n## original commit\ncommit 3628281d81d275f431bdc0da36fe7c7d66240175 (master)\nAuthor: Csaba Henk <>\nDate:   Fri May 4 23:56:51 2012 +0530\n\n    server: check msg size\n\n    Make sure client cannot crash server.\n\n    Q/A: check with PoC malicious client\n    by applying patch:\n\n    diff --git a/client.py b/client.py\n    --- a/client.py\n    +++ b/client.py\n    @@ -1,2 +1,2 @@\n     def client(conn):\n    -  print >> conn, \"hello\"\n    +  print >> conn, \"hell\"\n\n server.py |    4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\n## rebased commit\ncommit f5100d373242554f71db47592d8214c9e89d854a (bleeding-edge)\nAuthor: Csaba Henk <>\nDate:   Fri May 4 23:56:51 2012 +0530\n\n    server: check msg size\n\n    Make sure client cannot crash server.\n\n    Q/A: check with PoC malicious client\n    by applying patch:\n\n    diff --git a/client.py b/client.py\n    --- a/client.py\n    +++ b/client.py\n    @@ -1,2 +1,2 @@\n     def client(conn):\n    -  print >> conn, \"hello\"\n    +  print >> conn, \"hell\"\n\n client.py |    2 +-\n server.py |    4 +++-\n 2 files changed, 4 insertions(+), 2 deletions(-)\n\n## cherry-picked commit\ncommit c8fa20bd901d3bc0e745d976151d3b34d91f077d (HEAD)\nAuthor: Csaba Henk <>\nDate:   Fri May 4 23:56:51 2012 +0530\n\n    server: check msg size\n\n    Make sure client cannot crash server.\n\n    Q/A: check with PoC malicious client\n    by applying patch:\n\n    diff --git a/client.py b/client.py\n    --- a/client.py\n    +++ b/client.py\n    @@ -1,2 +1,2 @@\n     def client(conn):\n    -  print >> conn, \"hello\"\n    +  print >> conn, \"hell\"\n\n server.py |    4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\n\nCsaba\n"},{"id":"190783","messageId":"7vlil7ah5w.fsf@alter.siamese.dyndns.org","threadId":"30423","inReplyTo":"CAA-jfp=F7ije9nFJio5gnmNGptDR1ZTBbCi-aOv7JokqrXBgYA@mail.gmail.com","subject":"Re: commit message parsing error in rebase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-04T19:18:51Z","receivedAt":"2012-05-04T19:18:51Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sorry, but we do not support writing \"diff\" output in log message as-is\n(it is primarily because format-patch output from such a commit is hard to\nread for humans).  A common workaround is to indent such an illustrative\npatches indented in the log message.\n"},{"id":"190786","messageId":"CAA-jfp=yMVFrfFgh9A_zNC5nG8PTvNWBRpkbpbVK+7GUU7jO9Q@mail.gmail.com","threadId":"30423","inReplyTo":"7vlil7ah5w.fsf@alter.siamese.dyndns.org","subject":"Re: commit message parsing error in rebase","fromName":"Csaba Henk","fromEmail":"csaba@lowlife.hu","sentAt":"2012-05-04T19:24:21Z","receivedAt":"2012-05-04T19:24:21Z","isPatch":false,"sender":{"key":"csaba@gluster.com","avatar":"https://avatars.githubusercontent.com/u/43937?v=4"},"body":"On Sat, May 5, 2012 at 12:48 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Sorry, but we do not support writing \"diff\" output in log message as-is\n> (it is primarily because format-patch output from such a commit is hard to\n> read for humans).  A common workaround is to indent such an illustrative\n> patches indented in the log message.\n\nIn that case, would it not be wise to be explicit about that and warn\nor err if such a commit is attempted?\n\nCsaba\n"},{"id":"190788","messageId":"7vhavvag8a.fsf@alter.siamese.dyndns.org","threadId":"30423","inReplyTo":"CAA-jfp=yMVFrfFgh9A_zNC5nG8PTvNWBRpkbpbVK+7GUU7jO9Q@mail.gmail.com","subject":"Re: commit message parsing error in rebase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-04T19:39:01Z","receivedAt":"2012-05-04T19:39:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Csaba Henk <csaba@lowlife.hu> writes:\n\n> In that case, would it not be wise to be explicit about that and warn\n> or err if such a commit is attempted?\n\nI could see warning (but not erroring out or requiring -f) may work well.\n"}]}