{"thread":{"id":"52590","subject":"[RFC PATCH] unpack-trees: watch for out-of-range index position","startedAt":"2020-01-08T02:31:42Z","lastAt":"2020-01-10T23:07:14Z","messageCount":9,"participants":["Emily Shaffer","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"389443","messageId":"20200108023127.219429-1-emilyshaffer@google.com","threadId":"52590","inReplyTo":null,"subject":"[RFC PATCH] unpack-trees: watch for out-of-range index position","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-01-08T02:31:27Z","receivedAt":"2020-01-08T02:31:42Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"It's possible in a case where the index file contains a tree extension\nbut no blobs within that tree exist for index_pos_by_traverse_info() to\nsegfault. If the name_entry passed into index_pos_by_traverse_info() has\nno blobs inside, AND is alphabetically later than all blobs currently in\nthe index file, index_pos_by_traverse_info() will segfault. For example,\nan index file which looks something like this:\n\n  aaa#0\n  bbb/aaa#0\n  [Extensions]\n  TREE: zzz\n\nIn this example, 'index_name_pos(..., \"zzz/\", ...)' will return '-4',\nindicating that \"zzz/\" could be inserted at position 3. However, when\nthe checks which ensure that the insertion position of \"zzz/\" look for a\nblob at that position beginning with \"zzz/\", the index cache is accessed\nout of range, causing a segfault.\n\nThis kind of index state is not typically generated during user\noperations, and is in fact an edge case of the state being checked for\nin the conditional where it was added. However, since the entry for the\nBUG() line is ambiguous, tell some additional context to help Git\ndevelopers debug the failure later. When we know the name of the dir we\nwere trying to look up, it becomes possible to examine the index file\nin a hex util to determine what went wrong; the position gives a hint\nabout where to start looking.\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\n---\nThis issue came in via a bugreport from a user who had done some nasty\nthings like deleting various files in .git/ (and then couldn't remember\nhow they had done it). The concern was primarily that a segfault is ugly\nand scary, and possibly dangerous; I didn't see much problem with\nchecking for index-out-of-range if the result is a fatal error\nregardless.\n\nThe index file they shared for reproduction was very similar to the one\nthat I proposed in the commit message. However, though I had a repo\nwhere I could reproduce, since the user wasn't sure how they had gotten\nthere I struggled reasoning about how to produce these exact conditions.\nIt seems like during normal operation the index shouldn't learn about a\ntree extension where it doesn't know any blobs (in fact, I've become\nirritated before about being unable to stage/commit only directory\nstructure :) ).\n\nI also didn't find any test cases looking for the BUG() as it exists\nnow; I guess that's because BUG()s are supposed to be unreachable during\nnormal operation (or else they'd be a die()). So, it's marked RFC only\nbecause I couldn't think of a way to reliably reproduce or test this\nchange.\n\n - Emily\n\n unpack-trees.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex a75b068d33..d1ac1e179c 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -694,9 +694,11 @@ static int index_pos_by_traverse_info(struct name_entry *names,\n \tif (pos >= 0)\n \t\tBUG(\"This is a directory and should not exist in index\");\n \tpos = -pos - 1;\n-\tif (!starts_with(o->src_index->cache[pos]->name, name.buf) ||\n+\tif (pos >= o->src_index->cache_nr ||\n+\t    !starts_with(o->src_index->cache[pos]->name, name.buf) ||\n \t    (pos > 0 && starts_with(o->src_index->cache[pos-1]->name, name.buf)))\n-\t\tBUG(\"pos must point at the first entry in this directory\");\n+\t\tBUG(\"pos %d doesn't point to the first entry of %s in index\",\n+\t\t    pos, name.buf);\n \tstrbuf_release(&name);\n \treturn pos;\n }\n-- \n2.25.0.rc1.283.g88dfdc4193-goog\n\n"},{"id":"389453","messageId":"20200108071525.GB1675456@coredump.intra.peff.net","threadId":"52590","inReplyTo":"20200108023127.219429-1-emilyshaffer@google.com","subject":"Re: [RFC PATCH] unpack-trees: watch for out-of-range index position","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-08T07:15:25Z","receivedAt":"2020-01-08T07:15:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 07, 2020 at 06:31:27PM -0800, Emily Shaffer wrote:\n\n> This issue came in via a bugreport from a user who had done some nasty\n> things like deleting various files in .git/ (and then couldn't remember\n> how they had done it). The concern was primarily that a segfault is ugly\n> and scary, and possibly dangerous; I didn't see much problem with\n> checking for index-out-of-range if the result is a fatal error\n> regardless.\n>\n> [...]\n>  \tif (pos >= 0)\n>  \t\tBUG(\"This is a directory and should not exist in index\");\n>  \tpos = -pos - 1;\n> -\tif (!starts_with(o->src_index->cache[pos]->name, name.buf) ||\n> +\tif (pos >= o->src_index->cache_nr ||\n> +\t    !starts_with(o->src_index->cache[pos]->name, name.buf) ||\n>  \t    (pos > 0 && starts_with(o->src_index->cache[pos-1]->name, name.buf)))\n> -\t\tBUG(\"pos must point at the first entry in this directory\");\n> +\t\tBUG(\"pos %d doesn't point to the first entry of %s in index\",\n> +\t\t    pos, name.buf);\n\nThe new condition you added looks correct to me. I suspect this BUG()\nshould not be a BUG() at all, though. It's not necessarily a logic error\ninside Git, but as you showed it could indicate corrupt data we read\nfrom disk. The true is probably same of the \"pos >= 0\" condition checked\nabove.\n\nIt's mostly an academic distinction, though, as I think it would be\npretty reasonable for now to just die() here (eventually, though, we\nmight want to turn it into an error return).\n\n-Peff\n"},{"id":"389488","messageId":"xmqqeew93lfn.fsf@gitster-ct.c.googlers.com","threadId":"52590","inReplyTo":"20200108071525.GB1675456@coredump.intra.peff.net","subject":"Re: [RFC PATCH] unpack-trees: watch for out-of-range index position","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-08T17:30:36Z","receivedAt":"2020-01-08T17:30:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Tue, Jan 07, 2020 at 06:31:27PM -0800, Emily Shaffer wrote:\n>\n>> This issue came in via a bugreport from a user who had done some nasty\n>> things like deleting various files in .git/ (and then couldn't remember\n>> how they had done it). The concern was primarily that a segfault is ugly\n>> and scary, and possibly dangerous; I didn't see much problem with\n>> checking for index-out-of-range if the result is a fatal error\n>> regardless.\n>>\n>> [...]\n>>  \tif (pos >= 0)\n>>  \t\tBUG(\"This is a directory and should not exist in index\");\n>>  \tpos = -pos - 1;\n>> -\tif (!starts_with(o->src_index->cache[pos]->name, name.buf) ||\n>> +\tif (pos >= o->src_index->cache_nr ||\n>> +\t    !starts_with(o->src_index->cache[pos]->name, name.buf) ||\n>>  \t    (pos > 0 && starts_with(o->src_index->cache[pos-1]->name, name.buf)))\n>> -\t\tBUG(\"pos must point at the first entry in this directory\");\n>> +\t\tBUG(\"pos %d doesn't point to the first entry of %s in index\",\n>> +\t\t    pos, name.buf);\n>\n> The new condition you added looks correct to me. I suspect this BUG()\n> should not be a BUG() at all, though. It's not necessarily a logic error\n> inside Git, but as you showed it could indicate corrupt data we read\n> from disk. The true is probably same of the \"pos >= 0\" condition checked\n> above.\n\nIt does not sound like a BUG to me, either, but the new condition\ndoes look correct to me, too.  We can turn it into die() later if\nsomebody truly cares ;-)\n\nThanks, both.  Will queue.\n\n\n> It's mostly an academic distinction, though, as I think it would be\n> pretty reasonable for now to just die() here (eventually, though, we\n> might want to turn it into an error return).\n>\n> -Peff\n"},{"id":"389500","messageId":"20200108193833.GD181522@google.com","threadId":"52590","inReplyTo":"xmqqeew93lfn.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH] unpack-trees: watch for out-of-range index position","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-01-08T19:38:33Z","receivedAt":"2020-01-08T19:38:40Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Wed, Jan 08, 2020 at 09:30:36AM -0800, Junio C Hamano wrote:\n> Jeff King <peff@peff.net> writes:\n> \n> > On Tue, Jan 07, 2020 at 06:31:27PM -0800, Emily Shaffer wrote:\n> >\n> >> This issue came in via a bugreport from a user who had done some nasty\n> >> things like deleting various files in .git/ (and then couldn't remember\n> >> how they had done it). The concern was primarily that a segfault is ugly\n> >> and scary, and possibly dangerous; I didn't see much problem with\n> >> checking for index-out-of-range if the result is a fatal error\n> >> regardless.\n> >>\n> >> [...]\n> >>  \tif (pos >= 0)\n> >>  \t\tBUG(\"This is a directory and should not exist in index\");\n> >>  \tpos = -pos - 1;\n> >> -\tif (!starts_with(o->src_index->cache[pos]->name, name.buf) ||\n> >> +\tif (pos >= o->src_index->cache_nr ||\n> >> +\t    !starts_with(o->src_index->cache[pos]->name, name.buf) ||\n> >>  \t    (pos > 0 && starts_with(o->src_index->cache[pos-1]->name, name.buf)))\n> >> -\t\tBUG(\"pos must point at the first entry in this directory\");\n> >> +\t\tBUG(\"pos %d doesn't point to the first entry of %s in index\",\n> >> +\t\t    pos, name.buf);\n> >\n> > The new condition you added looks correct to me. I suspect this BUG()\n> > should not be a BUG() at all, though. It's not necessarily a logic error\n> > inside Git, but as you showed it could indicate corrupt data we read\n> > from disk. The true is probably same of the \"pos >= 0\" condition checked\n> > above.\n> \n> It does not sound like a BUG to me, either, but the new condition\n> does look correct to me, too.  We can turn it into die() later if\n> somebody truly cares ;-)\n> \n> Thanks, both.  Will queue.\n\nThanks much for the quick turnaround. If I hear more noise I'll give it\na try with die() or error code instead, but for now I'll move on to the\nnext bug on my list. :)\n\n - Emily\n"},{"id":"389503","messageId":"xmqqo8vd1yb2.fsf@gitster-ct.c.googlers.com","threadId":"52590","inReplyTo":"20200108193833.GD181522@google.com","subject":"Re: [RFC PATCH] unpack-trees: watch for out-of-range index position","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-08T20:35:29Z","receivedAt":"2020-01-08T20:35:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Emily Shaffer <emilyshaffer@google.com> writes:\n\n>> > The new condition you added looks correct to me. I suspect this BUG()\n>> > should not be a BUG() at all, though. It's not necessarily a logic error\n>> > inside Git, but as you showed it could indicate corrupt data we read\n>> > from disk. The true is probably same of the \"pos >= 0\" condition checked\n>> > above.\n>> \n>> It does not sound like a BUG to me, either, but the new condition\n>> does look correct to me, too.  We can turn it into die() later if\n>> somebody truly cares ;-)\n>> \n>> Thanks, both.  Will queue.\n>\n> Thanks much for the quick turnaround. If I hear more noise I'll give it\n> a try with die() or error code instead, but for now I'll move on to the\n> next bug on my list. :)\n\nBy the way, it is somewhat sad that we proceeded that far in the\nfirst place---such a corrupt on-disk index would have caused an\nearly die() if we did not get rid of the trailing-hash integrity\ncheck.\n"},{"id":"389519","messageId":"20200109075250.GA3978837@coredump.intra.peff.net","threadId":"52590","inReplyTo":"xmqqo8vd1yb2.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH] unpack-trees: watch for out-of-range index position","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-09T07:52:50Z","receivedAt":"2020-01-09T07:52:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 08, 2020 at 12:35:29PM -0800, Junio C Hamano wrote:\n\n> >> It does not sound like a BUG to me, either, but the new condition\n> >> does look correct to me, too.  We can turn it into die() later if\n> >> somebody truly cares ;-)\n> >> \n> >> Thanks, both.  Will queue.\n> >\n> > Thanks much for the quick turnaround. If I hear more noise I'll give it\n> > a try with die() or error code instead, but for now I'll move on to the\n> > next bug on my list. :)\n> \n> By the way, it is somewhat sad that we proceeded that far in the\n> first place---such a corrupt on-disk index would have caused an\n> early die() if we did not get rid of the trailing-hash integrity\n> check.\n\nPerhaps. The integrity check only protects against an index that was\nmodified after the fact, not one that was generated by a buggy Git. I'm\nnot sure we know how the index that led to this patch got into this\nstate (though it sounds like Emily has a copy and could check the hash\non it), but other cache-tree segfault I found recently was with an index\nwith an intact integrity hash.\n\nSo I think regardless of the trailing-hash check, we'd always want to be\ndefensive when reading on-disk data.\n\n-Peff\n"},{"id":"389554","messageId":"20200109224641.GF181522@google.com","threadId":"52590","inReplyTo":"20200109075250.GA3978837@coredump.intra.peff.net","subject":"Re: [RFC PATCH] unpack-trees: watch for out-of-range index position","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-01-09T22:46:41Z","receivedAt":"2020-01-09T22:46:49Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Thu, Jan 09, 2020 at 02:52:50AM -0500, Jeff King wrote:\n> On Wed, Jan 08, 2020 at 12:35:29PM -0800, Junio C Hamano wrote:\n> \n> > >> It does not sound like a BUG to me, either, but the new condition\n> > >> does look correct to me, too.  We can turn it into die() later if\n> > >> somebody truly cares ;-)\n> > >> \n> > >> Thanks, both.  Will queue.\n> > >\n> > > Thanks much for the quick turnaround. If I hear more noise I'll give it\n> > > a try with die() or error code instead, but for now I'll move on to the\n> > > next bug on my list. :)\n> > \n> > By the way, it is somewhat sad that we proceeded that far in the\n> > first place---such a corrupt on-disk index would have caused an\n> > early die() if we did not get rid of the trailing-hash integrity\n> > check.\n> \n> Perhaps. The integrity check only protects against an index that was\n> modified after the fact, not one that was generated by a buggy Git. I'm\n> not sure we know how the index that led to this patch got into this\n> state (though it sounds like Emily has a copy and could check the hash\n> on it), but other cache-tree segfault I found recently was with an index\n> with an intact integrity hash.\n\nYeah, I can do that, although I'm not sure how. The index itself is very\nsmall - it only contains one file and one tree extension - so I'll go\nahead and paste some poking and prodding, and if it's not what you\nwanted then please let me know what else to run.\n\n  $ g fsck --cache\n  Checking object directories: 100% (256/256), done.\n  Checking objects: 100% (20/20), done.\n  broken link from  commit 153a9a100eae7fdba5989ce39a5dd1782075517f\n                to  commit cca7ecaa5d8c398f41bfec7938cc6a526803579b\n  broken link from  commit 7d6bb91e31d18eadfaf855a9fb7ad6ba81b8b6d9\n                to  commit 03087a617bfe55f862cb1ef43273a2bd08e8b6d6\n  missing commit 03087a617bfe55f862cb1ef43273a2bd08e8b6d6\n  missing commit cca7ecaa5d8c398f41bfec7938cc6a526803579b\n  dangling commit 5e2c635433bc46b13061b276e481f63b1f6642c8\n\n  $ hexdump -C .git/index\n  00000000  44 49 52 43 00 00 00 02  00 00 00 01 5d 89 5e 22  |DIRC........].^\"|\n  00000010  23 bf a3 c4 5d 89 5e 22  23 bf a3 c4 00 00 fe 02  |#...].^\"#.......|\n  00000020  02 c8 f5 83 00 00 81 a4  00 06 c1 dc 00 01 5f 53  |.............._S|\n  00000030  00 00 06 b3 78 88 a4 f4  22 34 7d ad b0 c4 73 0f  |....x...\"4}...s.|\n  00000040  c5 bc f6 ea 1d 2d f0 3a  00 09 52 45 41 44 4d 45  |.....-.:..README|\n  00000050  2e 6d 64 00 54 52 45 45  00 00 00 3a 00 31 37 20  |.md.TREE...:.17 |\n  00000060  31 0a da 7f 67 25 40 7d  4e ce 9f d3 72 ce 4c e8  |1...g%@}N...r.L.|\n  00000070  40 6d 5d ad e9 79 67 69  74 6c 69 6e 74 00 34 20  |@m]..ygitlint.4 |\n  00000080  30 0a 93 63 25 17 69 e6  d6 92 78 97 55 4b 0f 8b  |0..c%.i...x.UK..|\n  00000090  ff a0 e8 2d 6d 71 32 d1  69 fc f2 38 42 f8 5a 6e  |...-mq2.i..8B.Zn|\n  000000a0  05 35 d6 94 41 c0 9f c7  ba 43                    |.5..A....C|\n  000000aa\n\n  $ find .git/objects -type f\n  .git/objects/pack/pack-5e5d5e7c3cbd60a99b4c0295a2935885fbb235a1.idx\n  .git/objects/pack/pack-5e5d5e7c3cbd60a99b4c0295a2935885fbb235a1.pack\n  .git/objects/15/3a9a100eae7fdba5989ce39a5dd1782075517f\n  .git/objects/5e/2c635433bc46b13061b276e481f63b1f6642c8\n\n(By the way, cat-file barfs on both those loose objects, for the reason fsck\nreveals.)\n\nI hope that's helpful.\n\n - Emily\n"},{"id":"389566","messageId":"20200110063741.GA409153@coredump.intra.peff.net","threadId":"52590","inReplyTo":"20200109224641.GF181522@google.com","subject":"Re: [RFC PATCH] unpack-trees: watch for out-of-range index position","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-10T06:37:41Z","receivedAt":"2020-01-10T06:37:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 09, 2020 at 02:46:41PM -0800, Emily Shaffer wrote:\n\n> > Perhaps. The integrity check only protects against an index that was\n> > modified after the fact, not one that was generated by a buggy Git. I'm\n> > not sure we know how the index that led to this patch got into this\n> > state (though it sounds like Emily has a copy and could check the hash\n> > on it), but other cache-tree segfault I found recently was with an index\n> > with an intact integrity hash.\n> \n> Yeah, I can do that, although I'm not sure how. The index itself is very\n> small - it only contains one file and one tree extension - so I'll go\n> ahead and paste some poking and prodding, and if it's not what you\n> wanted then please let me know what else to run.\n\nI was thinking you would run something like:\n\n  size=$(stat --format=%s \"$file\")\n  actual=$(head -c $(($size-20)) \"$file\" | sha1sum | awk '{print $1}')\n  expect=$(xxd -s -20 -g 20 -c 20 \"$file\" | awk '{print $2}')\n  if test \"$actual\" = \"$expect\"; then\n          echo \"OK ($actual)\"\n  else\n          echo \"FAIL ($actual != $expect)\"\n  fi\n\nto manually check the sha1. But...\n\n>   $ g fsck --cache\n>   Checking object directories: 100% (256/256), done.\n>   Checking objects: 100% (20/20), done.\n>   broken link from  commit 153a9a100eae7fdba5989ce39a5dd1782075517f\n>                 to  commit cca7ecaa5d8c398f41bfec7938cc6a526803579b\n>   broken link from  commit 7d6bb91e31d18eadfaf855a9fb7ad6ba81b8b6d9\n>                 to  commit 03087a617bfe55f862cb1ef43273a2bd08e8b6d6\n>   missing commit 03087a617bfe55f862cb1ef43273a2bd08e8b6d6\n>   missing commit cca7ecaa5d8c398f41bfec7938cc6a526803579b\n>   dangling commit 5e2c635433bc46b13061b276e481f63b1f6642c8\n\n...fsck would have reported a problem there, since we explicitly kept\nthe check there in a33fc72fe9 (read-cache: force_verify_index_checksum,\n2017-04-14).\n\nAnd just to be double-sure, I used this:\n\n>   $ hexdump -C .git/index\n>   00000000  44 49 52 43 00 00 00 02  00 00 00 01 5d 89 5e 22  |DIRC........].^\"|\n>   00000010  23 bf a3 c4 5d 89 5e 22  23 bf a3 c4 00 00 fe 02  |#...].^\"#.......|\n>   00000020  02 c8 f5 83 00 00 81 a4  00 06 c1 dc 00 01 5f 53  |.............._S|\n>   00000030  00 00 06 b3 78 88 a4 f4  22 34 7d ad b0 c4 73 0f  |....x...\"4}...s.|\n>   00000040  c5 bc f6 ea 1d 2d f0 3a  00 09 52 45 41 44 4d 45  |.....-.:..README|\n>   00000050  2e 6d 64 00 54 52 45 45  00 00 00 3a 00 31 37 20  |.md.TREE...:.17 |\n>   00000060  31 0a da 7f 67 25 40 7d  4e ce 9f d3 72 ce 4c e8  |1...g%@}N...r.L.|\n>   00000070  40 6d 5d ad e9 79 67 69  74 6c 69 6e 74 00 34 20  |@m]..ygitlint.4 |\n>   00000080  30 0a 93 63 25 17 69 e6  d6 92 78 97 55 4b 0f 8b  |0..c%.i...x.UK..|\n>   00000090  ff a0 e8 2d 6d 71 32 d1  69 fc f2 38 42 f8 5a 6e  |...-mq2.i..8B.Zn|\n>   000000a0  05 35 d6 94 41 c0 9f c7  ba 43                    |.5..A....C|\n>   000000aa\n\nto reconstruct the file and check its sha1, and indeed it is fine.\n\nSo this bogus index was probably actually created by Git, not an\nafter-the-fact byte corruption.\n\n-Peff\n"},{"id":"389600","messageId":"20200110230706.GH181522@google.com","threadId":"52590","inReplyTo":"20200110063741.GA409153@coredump.intra.peff.net","subject":"Re: [RFC PATCH] unpack-trees: watch for out-of-range index position","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2020-01-10T23:07:06Z","receivedAt":"2020-01-10T23:07:14Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Fri, Jan 10, 2020 at 01:37:41AM -0500, Jeff King wrote:\n> On Thu, Jan 09, 2020 at 02:46:41PM -0800, Emily Shaffer wrote:\n> \n> > > Perhaps. The integrity check only protects against an index that was\n> > > modified after the fact, not one that was generated by a buggy Git. I'm\n> > > not sure we know how the index that led to this patch got into this\n> > > state (though it sounds like Emily has a copy and could check the hash\n> > > on it), but other cache-tree segfault I found recently was with an index\n> > > with an intact integrity hash.\n> > \n> > Yeah, I can do that, although I'm not sure how. The index itself is very\n> > small - it only contains one file and one tree extension - so I'll go\n> > ahead and paste some poking and prodding, and if it's not what you\n> > wanted then please let me know what else to run.\n> \n> I was thinking you would run something like:\n> \n>   size=$(stat --format=%s \"$file\")\n>   actual=$(head -c $(($size-20)) \"$file\" | sha1sum | awk '{print $1}')\n>   expect=$(xxd -s -20 -g 20 -c 20 \"$file\" | awk '{print $2}')\n>   if test \"$actual\" = \"$expect\"; then\n>           echo \"OK ($actual)\"\n>   else\n>           echo \"FAIL ($actual != $expect)\"\n>   fi\n> \n> to manually check the sha1.\n\nUnsurprising given your mail, yeah, this looks OK when I run it against\nthe repo in question.\n\n> So this bogus index was probably actually created by Git, not an\n> after-the-fact byte corruption.\n\nDisappointingly, the repro repo we got was aggressively redacted - I\ndon't have any reflogs to look through and try and get a hint of what\nhappened, and I imagine the reporter has moved on with their life enough\nthat we can't get something useful from there now.\n\n - Emily\n"}]}