{"thread":{"id":"41373","subject":"[PATCH] test-path-utils: use xsnprintf in favor of strcpy","startedAt":"2016-02-08T22:21:55Z","lastAt":"2016-02-09T10:13:12Z","messageCount":7,"participants":["Jeff King","Junio C Hamano","Eric Wong","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"277778","messageId":"20160208222155.GA17395@sigill.intra.peff.net","threadId":"41373","inReplyTo":null,"subject":"[PATCH] test-path-utils: use xsnprintf in favor of strcpy","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-08T22:21:55Z","receivedAt":"2016-02-08T22:21:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This strcpy will never overflow because it's copying from\nbaked-in test data. But we would prefer to avoid strcpy\nentirely, as it makes it harder to audit for real security\nbugs.\n\nSigned-off-by: Jeff King \n---\nRepost of <20160114202608.GA8806@sigill.intra.peff.net> from a few weeks\nago (sorry, gmane is down so I can't generate a link). I think the\noriginal was never applied because the topic that introduced the strcpy\n(js/dirname-basename) predated xsnprintf, so there was some merging\ncomplexity. Now that topic is in master, so this can be applied there.\n\n test-path-utils.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/test-path-utils.c b/test-path-utils.c\nindex c3adcd8..6232dfe 100644\n--- a/test-path-utils.c\n+++ b/test-path-utils.c\n@@ -56,7 +56,7 @@ static int test_function(struct test_data *data, char *(*func)(char *input),\n \t\tif (!data[i].from)\n \t\t\tto = func(NULL);\n \t\telse {\n-\t\t\tstrcpy(buffer, data[i].from);\n+\t\t\txsnprintf(buffer, sizeof(buffer), \"%s\", data[i].from);\n \t\t\tto = func(buffer);\n \t\t}\n \t\tif (!strcmp(to, data[i].to))\n-- \n2.7.1.526.gd04f550\n"},{"id":"277780","messageId":"20160208222501.GA21671@sigill.intra.peff.net","threadId":"41373","inReplyTo":"20160208222155.GA17395@sigill.intra.peff.net","subject":"[PATCH] rerere: replace strcpy with xsnprintf","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-08T22:25:01Z","receivedAt":"2016-02-08T22:25:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This shouldn't overflow, as we are copying a sha1 hex into a\n41-byte buffer. But it does not hurt to use a bound-checking\nfunction, which protects us and makes auditing for overflows\neasier.\n\nSigned-off-by: Jeff King \n---\nThese strcpy calls go away in jc/rerere-multi, so I was holding onto\nthis to see if that graduated. But since that is stalled, I figured it\ncannot hurt to post (and the conflict resolution is obviously trivial).\n\nWith this and the previous patch, it makes our code base strcpy free.\nYay.\n\n rerere.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex 403c700..587b7e2 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -48,7 +48,7 @@ static int has_rerere_resolution(const struct rerere_id *id)\n static struct rerere_id *new_rerere_id_hex(char *hex)\n {\n \tstruct rerere_id *id = xmalloc(sizeof(*id));\n-\tstrcpy(id->hex, hex);\n+\txsnprintf(id->hex, sizeof(id->hex), \"%s\", hex);\n \treturn id;\n }\n \n@@ -904,7 +904,7 @@ int rerere_forget(struct pathspec *pathspec)\n static struct rerere_id *dirname_to_id(const char *name)\n {\n \tstatic struct rerere_id id;\n-\tstrcpy(id.hex, name);\n+\txsnprintf(id.hex, sizeof(id.hex), \"%s\", name);\n \treturn &id;\n }\n \n-- \n2.7.1.526.gd04f550\n"},{"id":"277783","messageId":"xmqqh9hi6d00.fsf@gitster.mtv.corp.google.com","threadId":"41373","inReplyTo":"20160208222155.GA17395@sigill.intra.peff.net","subject":"Re: [PATCH] test-path-utils: use xsnprintf in favor of strcpy","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-08T22:41:51Z","receivedAt":"2016-02-08T22:41:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> This strcpy will never overflow because it's copying from\n> baked-in test data. But we would prefer to avoid strcpy\n> entirely, as it makes it harder to audit for real security\n> bugs.\n>\n> Signed-off-by: Jeff King \n> ---\n> Repost of <20160114202608.GA8806@sigill.intra.peff.net> from a few weeks\n> ago (sorry, gmane is down so I can't generate a link). I think the\n> original was never applied because the topic that introduced the strcpy\n> (js/dirname-basename) predated xsnprintf, so there was some merging\n> complexity. Now that topic is in master, so this can be applied there.\n\nThanks. This kind of considerate \"holding it off for a few weeks\"\nhelps things a lot.\n\n>\n> test-path-utils.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/test-path-utils.c b/test-path-utils.c\n> index c3adcd8..6232dfe 100644\n> --- a/test-path-utils.c\n> +++ b/test-path-utils.c\n> @@ -56,7 +56,7 @@ static int test_function(struct test_data *data, char *(*func)(char *input),\n> \t\tif (!data[i].from)\n> \t\t\tto = func(NULL);\n> \t\telse {\n> -\t\t\tstrcpy(buffer, data[i].from);\n> +\t\t\txsnprintf(buffer, sizeof(buffer), \"%s\", data[i].from);\n> \t\t\tto = func(buffer);\n> \t\t}\n> \t\tif (!strcmp(to, data[i].to))\n"},{"id":"277786","messageId":"20160208230726.GB3487@dcvr.yhbt.net","threadId":"41373","inReplyTo":"20160208222155.GA17395@sigill.intra.peff.net","subject":"Re: [PATCH] test-path-utils: use xsnprintf in favor of strcpy","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2016-02-08T23:07:26Z","receivedAt":"2016-02-08T23:07:26Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Jeff King wrote:\n> Repost of <20160114202608.GA8806@sigill.intra.peff.net> from a few weeks\n> ago (sorry, gmane is down so I can't generate a link).\n\nI prefer we use links derived from Message-IDs anyways. This\nprevents reliance on gmane article numbers being a central point\nof failure:\n\nhttp://marc.info/?i=$MESSAGE_ID\nhttp://mid.gmane.org/$MESSAGE_ID\nhttp://mid.mail-archive.com/$MESSAGE_ID\n\nBut the MESSAGE_ID above seems missing from mail-archive.com, in this case.\n\nMaybe there's more lookup-by-Message-ID services out there...\n"},{"id":"277787","messageId":"xmqqd1s66bt8.fsf@gitster.mtv.corp.google.com","threadId":"41373","inReplyTo":"20160208222501.GA21671@sigill.intra.peff.net","subject":"Re: [PATCH] rerere: replace strcpy with xsnprintf","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-08T23:07:31Z","receivedAt":"2016-02-08T23:07:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> This shouldn't overflow, as we are copying a sha1 hex into a\n> 41-byte buffer. But it does not hurt to use a bound-checking\n> function, which protects us and makes auditing for overflows\n> easier.\n>\n> Signed-off-by: Jeff King \n> ---\n> These strcpy calls go away in jc/rerere-multi, so I was holding onto\n> this to see if that graduated. But since that is stalled, I figured it\n> cannot hurt to post (and the conflict resolution is obviously trivial).\n>\n> With this and the previous patch, it makes our code base strcpy free.\n> Yay.\n\nThanks. I think jc/rerere-multi can be rerolled on top of this.\n\n>\n> rerere.c | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/rerere.c b/rerere.c\n> index 403c700..587b7e2 100644\n> --- a/rerere.c\n> +++ b/rerere.c\n> @@ -48,7 +48,7 @@ static int has_rerere_resolution(const struct rerere_id *id)\n> static struct rerere_id *new_rerere_id_hex(char *hex)\n> {\n> \tstruct rerere_id *id = xmalloc(sizeof(*id));\n> -\tstrcpy(id->hex, hex);\n> +\txsnprintf(id->hex, sizeof(id->hex), \"%s\", hex);\n> \treturn id;\n> }\n> \n> @@ -904,7 +904,7 @@ int rerere_forget(struct pathspec *pathspec)\n> static struct rerere_id *dirname_to_id(const char *name)\n> {\n> \tstatic struct rerere_id id;\n> -\tstrcpy(id.hex, name);\n> +\txsnprintf(id.hex, sizeof(id.hex), \"%s\", name);\n> \treturn &id;\n> }\n"},{"id":"277788","messageId":"20160208231332.GA23638@sigill.intra.peff.net","threadId":"41373","inReplyTo":"20160208230726.GB3487@dcvr.yhbt.net","subject":"Re: [PATCH] test-path-utils: use xsnprintf in favor of strcpy","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-08T23:13:32Z","receivedAt":"2016-02-08T23:13:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 08, 2016 at 11:07:26PM +0000, Eric Wong wrote:\n\n> Jeff King wrote:\n> > Repost of <20160114202608.GA8806@sigill.intra.peff.net> from a few weeks\n> > ago (sorry, gmane is down so I can't generate a link).\n> \n> I prefer we use links derived from Message-IDs anyways. This\n> prevents reliance on gmane article numbers being a central point\n> of failure:\n> \n> http://marc.info/?i=$MESSAGE_ID\n> http://mid.gmane.org/$MESSAGE_ID\n> http://mid.mail-archive.com/$MESSAGE_ID\n\nI actually do, too. I keep a local archive of the whole list, and I have\na script that hits gmane to convert their article ids into message-ids.\nWhen gmane is down I can still use my archive, but I can't resolve\nanybody's article mentions. :)\n\nI mostly use gmane links because people are used to them, though (I also\ndon't think there's a way using message-ids to point to a whole thread\nwith an article highlighted, though of course readers can get their by\nclicking through).\n\n> But the MESSAGE_ID above seems missing from mail-archive.com, in this case.\n\nIt seems to have a giant hole in git@vger messages between 2016-01-07\nand 2016-01-20, which covers the referenced message (which was on the\n14th).\n\n-Peff\n"},{"id":"277799","messageId":"alpine.DEB.2.20.1602091112570.2964@virtualbox","threadId":"41373","inReplyTo":"20160208222155.GA17395@sigill.intra.peff.net","subject":"Re: [PATCH] test-path-utils: use xsnprintf in favor of strcpy","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-02-09T10:13:12Z","receivedAt":"2016-02-09T10:13:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Mon, 8 Feb 2016, Jeff King wrote:\n\n> This strcpy will never overflow because it's copying from\n> baked-in test data. But we would prefer to avoid strcpy\n> entirely, as it makes it harder to audit for real security\n> bugs.\n> \n> Signed-off-by: Jeff King \n\nACK, of course.\n\nThanks!\nDscho\n"}]}