On Tue, 2014-04-29 at 18:55:35 +0200, Jakub Wilk wrote: > * Guillem Jover <[email protected]>, 2014-04-29, 08:11: > >1. Simply revert the patch, and ignore issues w/ partial upgrades (at > >least for now?). > >2. Revert the patch and add versioned depdendencies against the working > >patch package. This might require some dist-upgrade tests, though. > >3. Fix the patch to take into account the old behaviour, by checking if > >either of the filenames (escaped and unescaped) are unsafe. > > > >I guess the last one is the “safest option”. > > For a quick fix, 3 is probably the best.
Did you mean 1? After having checked to implement 3, there's many parts of the code that need to be changed and moved around, I'll try to cook an actual patch to see how bad it is though. There's also the newly supported git formatted patches now recognized by patch, “fortunately” Dpkg::Source::Patch does not understand them and because it creates any necessary directories (or what looks like one), I don't see a way it can be exploited. But I might be short on imagination at this moment. Also another issue is that independently of partial upgrades, dpkg-dev might be used on other systems where we don't know what patch version is available, so it could be vulnerable or not depending on that. :/ > But I think this bug shows that validating diffs is not viable in the long > run. We need to either fix patch(1) not to traverse directory symlinks, Yeah, I've to agree with that, the only really safe option here is for patch to not follow any symlinks at all. And that seems to have been partly the intention behind the new default and the option to revert it with --follow-symlinks, but it seems to only apply to the last component. A patch program that follows symlinks also means that extracting a Debian source package manually is unsafe… > or implement a completely different strategy: > > 1) Unpack .orig.tar. > 2) Delete all symlinks (and maybe also other non-regular files). > 3) Apply patches. > 4) Restore all the files deleted in step 2. > > > In another mail Javier suggested to check --dry-run output. I don't think > this is feasible. Parsing --dry-run output is probably even harder than > parsing patches, Yeah, I don't think parsing --dry-run output is a good idea either, it's not future-proof and prone to break if the output changes. Your proposed solution might be the safer and more future-proof way to deal with the current patch changing behaviour, but it might not cover all future new features (think similar extensions like the git formatted patches). Thanks, Guillem -- To UNSUBSCRIBE, email to [email protected] with a subject of "unsubscribe". Trouble? Contact [email protected]
