appc / docker2aci

library and CLI tool to convert Docker images to ACIs (archived, see https://github.com/rkt/rkt/issues/4024)
Apache License 2.0
186 stars 60 forks source link

a malicious ACI created during layer archive extraction via symlink or hardlink attack #205

Closed soh0ro0t closed 7 years ago

soh0ro0t commented 8 years ago

Hi, recently, i reviewed the code in docker2aci and found some interesting stuff.There is a potential threat vector that was discovered during layer archive extraction, which could cause a milicious ACI-formatted image contains symlink or hardlink to a relative path out of rootfs. It seems that ValidateACI only check whether the internal files's prefix is "rootfs", without essential review for the Linkname of some link file, it may perform some critical behaviors, like breakout etc, i think it is possible that a malicious link file to the sensitive dir or file in the layer archive. for example, a crafted tar file header is like this:

{
    {
            header: &tar.Header{
                Name:     "looplink",
                Typeflag: tar.TypeSymlink,
                Linkname: "../../../etc/init.d"
            },
    },
    {
            header: &tar.Header{
                Name: "looplink/havefun",
            },
    },
}

what do you think about this issue ?

lucab commented 8 years ago

Thanks for the report @TheBeeMan. The code snippet you pointed out seems to lack validation of links target, I agree it can do better there. I have to check if additional validation is performed at a different layer but so far from a preliminary analysis I haven't seen any.

From what I have observed so far, writeACI() is never involved in extraction to filesystem (copy/validation is performed via in-memory readers) so I'm wondering if you have a PoC or an attack scenario directly against docker2aci, or if it is a more generic concern regarding the conversion of crafted images.

Another open question: ValidateACI() mostly relies on ValidateArchive() from appc/spec, so I'm wondering if this additional validation should be better performed directly there instead. @jonboulle?

soh0ro0t commented 8 years ago

I prefer to a more generic concern regarding the conversion of crafted images, there is no PoC directly against docker2aci, as CVE Assignment Team said:

docker2aci is apparently a library [...] and we almost always recognize the potential for an unattended use case for any library. [...] Someone can call the xx function from an arbitrary application. [...] It might be automated with cron or a similar unattended tool that runs in an unrestricted (non-container) environment. Thus, there is an availability impact because [...]

docker2aci uses writeACI() to generate a malicious ACI-formatted image, how could you guarantee others not to extract it to filesystem from an arbitrary application ?

I think a CVE deserved, right ?

soh0ro0t commented 8 years ago

could you submit the issue to CVE Assignment Team for review, and request a CVE if possible ?

jonboulle commented 8 years ago

@TheBeeMan the onus is on extractors to safely unpack the archive. Symlinks are resolved at path resolution time and container images are intended to be executed inside of a chroot jail; accordingly, it's quite valid to have relative symlinks that point "outside" the archive (or absolute links that on first glance might appear to reference the host - for example, var/run --> /run). Consequently you'll see that implementations like Docker's or rkt's will perform the extraction itself in a chroot jail.

Analogously, one can trivially create such a "malicious" image using GNU tar, and we can't guarantee that arbitrary applications aren't using tar (as a library or executable) to extract such images - but this doesn't mean we should file a CVE against tar.

Does that make sense?

soh0ro0t commented 8 years ago

fine

lucab commented 7 years ago

Not a docker2aci bug, thus closing.