{"thread":{"id":"11819","subject":"[PATCH] Avoid segfault when passed malformed refspec","startedAt":"2008-02-02T00:00:13Z","lastAt":"2008-02-02T01:26:02Z","messageCount":3,"participants":["Sean","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"67053","messageId":"BAYC1-PASMTP124F1019C2D2CD7AA81CF5AE310@CEZ.ICE","threadId":"11819","inReplyTo":null,"subject":"[PATCH] Avoid segfault when passed malformed refspec","fromName":"Sean","fromEmail":"seanlkml@sympatico.ca","sentAt":"2008-02-02T00:00:13Z","receivedAt":"2008-02-02T00:00:13Z","isPatch":true,"sender":{"key":"seanlkml@sympatico.ca","avatar":"https://gravatar.com/avatar/f92923f54fc08c401fc59b71829d4b89e9b8087fbba45ff87c82e6a83aee02ae?d=mp&s=160"},"body":"\nA refspec typo can cause a Null-pointer dereference and segmentation\nfault. For instance, the space before the colon in the following\nexample results in a segfault:\n\n $ git fetch ../repo refs/heads/* :refs/heads/*\n Segmentation fault (core dumped)\n\nTo avoid the segfault, set an empty refspec destination string\nif one isn't found by parsing.\n\nSigned-off-by: Sean Estabrooks \n---\n remote.c | 2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 0e00680..414c73a 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -336,6 +336,8 @@ struct refspec *parse_ref_spec(int nr_refspec, const char **refspec)\n \t\t\tep = gp;\n \t\t}\n \t\trs[i].src = xstrndup(sp, ep - sp);\n+\t\tif (!rs[i].dst)\n+\t\t\trs[i].dst = xstrdup(\"\");\n \t}\n \treturn rs;\n }\n-- \n1.5.4.rc5.20.g4b806\n"},{"id":"67056","messageId":"7vzluk6ugn.fsf@gitster.siamese.dyndns.org","threadId":"11819","inReplyTo":"BAYC1-PASMTP124F1019C2D2CD7AA81CF5AE310@CEZ.ICE","subject":"Re: [PATCH] Avoid segfault when passed malformed refspec","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-02T01:03:04Z","receivedAt":"2008-02-02T01:03:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sean writes:\n\n> A refspec typo can cause a Null-pointer dereference and segmentation\n> fault. For instance, the space before the colon in the following\n> example results in a segfault:\n>\n> $ git fetch ../repo refs/heads/* :refs/heads/*\n> Segmentation fault (core dumped)\n>\n> To avoid the segfault, set an empty refspec destination string\n> if one isn't found by parsing.\n>\n> Signed-off-by: Sean Estabrooks \n> ---\n> remote.c | 2 ++\n> 1 files changed, 2 insertions(+), 0 deletions(-)\n>\n> diff --git a/remote.c b/remote.c\n> index 0e00680..414c73a 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -336,6 +336,8 @@ struct refspec *parse_ref_spec(int nr_refspec, const char **refspec)\n> \t\t\tep = gp;\n> \t\t}\n> \t\trs[i].src = xstrndup(sp, ep - sp);\n> +\t\tif (!rs[i].dst)\n> +\t\t\trs[i].dst = xstrdup(\"\");\n> \t}\n> \treturn rs;\n> }\n\nI haven't followed the codepath carefully before responding, it\nfeels like sweeping the breakage under the carpet, without\nfixing the real issue.\n\nIf the problem is a badly formatted input, shouldn't the code\ndie loudly with diagnostic message, instead of pretending as if\nthe user said something different (and sensible), especially\nwithout telling the user that that is what the code is doing?\n"},{"id":"67059","messageId":"BAYC1-PASMTP11F54ED60A103C52F2406AAE310@CEZ.ICE","threadId":"11819","inReplyTo":"7vzluk6ugn.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Avoid segfault when passed malformed refspec","fromName":"Sean","fromEmail":"seanlkml@sympatico.ca","sentAt":"2008-02-02T01:26:02Z","receivedAt":"2008-02-02T01:26:02Z","isPatch":true,"sender":{"key":"seanlkml@sympatico.ca","avatar":"https://gravatar.com/avatar/f92923f54fc08c401fc59b71829d4b89e9b8087fbba45ff87c82e6a83aee02ae?d=mp&s=160"},"body":"On Fri, 01 Feb 2008 17:03:04 -0800\nJunio C Hamano wrote:\n\n> Sean writes:\n> >\n> > +\t\tif (!rs[i].dst)\n> > +\t\t\trs[i].dst = xstrdup(\"\");\n>\n> I haven't followed the codepath carefully before responding, it\n> feels like sweeping the breakage under the carpet, without\n> fixing the real issue.\n> \n> If the problem is a badly formatted input, shouldn't the code\n> die loudly with diagnostic message, instead of pretending as if\n> the user said something different (and sensible), especially\n> without telling the user that that is what the code is doing?\n> \n\nHey Junio,\n\nYou're probably right. It seemed like a reasonable fix at the time\nwithout having to understand the code too deeply. With the above\npatch, the code does complain to the user:\n\n $ git-fetch ../repo refs/heads/* :refs/heads/* \n fatal: * refusing to create funny ref 'floop' locally\n\nBut surely a better error could be shown if fetch is made to\nsquawk whenever a destination ref is omitted. I just wasn't\nconfident enough in the code, or in knowing what refspec rules\nare universally applicable.\n\nSean\n"}]}