On Fri, Aug 18, 2017 at 01:30:23PM +0200, Patryk Obara wrote: > > We'd reject such an input totally (though as an interesting side effect, > > you can convince the parser to allocate 20x as much RAM as you send it; > > one oid for each space). > > Grafts are not populated during clone operation, so it really would be user > making his life miserable. I could allocate FLEXI_ARRAY of size > min(n, line->len / (GIT_*MIN*_HEXSZ+1)) instead… but I think it's not even > worth the cost of making the code more complicated (and I don't want > to reintroduce these size macros in here. > > We _could_ put an artificial limit on graft parents, though (e.g. 10) and > display an error message urging the user to stop using grafts?
Yeah, sorry, I should have made more clear that this is fine. I always try to read parsing code with my paranoid hat on, but I agree that grafts aren't really exposed to untrusted entities. In general I'd prefer to avoid artificial limits unless there's a need for them. There are already spots (like receive-pack) where you can ask Git to store bytes in RAM as fast as you can send them. What I found interesting about this one was the 20:1 amplification. :) > Before sending v3 I tried two other alternative implementations (perhaps I > should've listed them in the v3 cover letter): It might even be worth listing them in the commit message. Somebody finding your commit 3 years from via "git log -S" or "git blame" might say "yes, but why didn't they just do it like...". You can respond to them preemptively. :) -Peff
