Antoine Beaupré wrote: > It was challenging work, especially for the two main patches. > > The first big patch is "limit url downloads to whitelisted schemes": > > http://source.git-annex.branchable.com/?p=source.git;a=commitdiff;h=28720c795ff57a55b48e56d15f9b6bcb977f48d9 > > The main challenge with this patch is that `getUrlInfo` is quite different > in 5.x: it's called `exists` and has a somewhat different structure. I > think I've been able to port it correctly, but reviewers will want to > pay special attention to that function in Utility/Url.hs. It looks ok. > More concerning are chunks affecting code which does a > `verifyKeyContent`. There is no equivalent in 5.x and I am worried this > was used to also check for bad URLs as a side-effect. More critically, > I'm concerned that the lack of such infrastructure makes it impossible > to fix CVE-2018-10859 in the 5.x branch without backporting a > significant part of that work. It seems to me the patchset I have now > does not actually have provisions to restrict restriction to certain > content. I agree, verifyKeyContent is an essential part of the security fix. As described in https://git-annex.branchable.com/security/CVE-2018-10857_and_CVE-2018-10859/ CVE-2018-10859 was fixed by making git-annex refuse to download encrypted content from special remotes, unless it knows the hash of the expected content. When the attacker provides some other gpg encrypted content, it will fail the hash check and be discarded. So I think that backporting 2fb3722ce993f01adb3ea9d5f8855c3e6a99c249 is a prerequisite of your backport, though whether you'd want to expose the annex.verify setting is up to you. > I am worried about Get.hs, where I had to hardcode a > RetrievalAllKeysSecure policy in case no remote is specified, since I > couldn't figure out how to find the remote at that level. I similarly > shot from the hip, in the dark, with Sync.hs, unfortunately. I can confirm you left it insecure. Take the Command/Get.hs and Command/Sync.hs hunks from 2fb3722ce993f01adb3ea9d5f8855c3e6a99c249 which move the getViaTmp into a scope where you have a Remote to access. -- see shy jo
Attachment:
signature.asc
Description: PGP signature