Skip to content
This repository was archived by the owner on Aug 14, 2020. It is now read-only.

Infinite loop vulnerability in retrieving images chain #203

Closed
soh0ro0t opened this issue Sep 30, 2016 · 7 comments · Fixed by #204
Closed

Infinite loop vulnerability in retrieving images chain #203

soh0ro0t opened this issue Sep 30, 2016 · 7 comments · Fixed by #204

Comments

@soh0ro0t
Copy link

soh0ro0t commented Sep 30, 2016

Hi,

In code reviewing, i found an infinite loop vulnerability in retrieving images chain using docker2aci, it occurs during the corresponding json file parsing from user's image archive, fetching the parent image ID until ID is nil. There must be a possibility that the images chain may be a closed cycle, thus , docker2aci will fall into an infinite loop, that's indeed true by some interesting tests.

I think the core cause of this issue is lacking in essential check for duplicated image ID, such as the current image ID could not be equal to its parent image ID, most important, check whether the images chain is a closed cycle.

I processed some interesting test for this issue, building a crafted image whose top layer's parent ID points to itself, then an infinite loop occurred, this flaw caused excessive CPU cycles & resources consume on the host.

expecting subsequent discuss and fix the issue together, and could you request a CVE identifier for that ?

@lucab
Copy link
Contributor

lucab commented Oct 7, 2016

Thanks for the report @thebeeman. From the description above, even if not explicitly stated it looks like your code review is covering the logic in getAncestry() (please pinpoint if this covers something else instead).

In the above code, I agree that the current parent-handling behavior is buggy as it doesn't take into account cyclic dependencies.

However I'm not completely sure about any security implication of this. Triggering this infinite loop seems to require local access and only concern a single user process. This will just result in a never-ending conversion of a single malformed image, which shouldn't impact availability per-se as there is no single service to DoS here. As you pointed out, there will be indeed some over-usage of processing resources but this is something that is typically controlled by the OS (eg. via cgroups, priority-throttling, OOM-killing).

What's was your original impact analysis of this suggesting that it requires a CVE?

@soh0ro0t
Copy link
Author

soh0ro0t commented Oct 9, 2016

This is a typical denial of service vulnerability, which could result in over-usage of processing resources. I think it is a CVE worthy vulnerability, although it is a low severity issue but a crafted image could cause docker2aci fall into a never-ending conversion.

The way you think about security is different with me, denial of service is a grey area, Obviously if I send 10 gigabits of request traffic or an valid input files and the processor gets slow/non responsive the CVE response would be "No CVE for you" ,but if a single crafted file prevents the whole system from working in an expected manner, that may be a problem that is worth a CVE.

Just for reference, infinite loop is a well-known security issue (CWE-835)

@soh0ro0t
Copy link
Author

Could you request a CVE for that ?

@lucab
Copy link
Contributor

lucab commented Oct 10, 2016

Sure! I've just forwarded this to CVE Assignment Team at MITRE and put you in copy. We'll be waiting for their reply regarding submission review and CVE ID allocation. Thanks again for following up on this.

@lucab
Copy link
Contributor

lucab commented Oct 13, 2016

After MITRE review, in the context of docker2aci usage as an embedded library, this bug has been recognized as a security issue and assigned CVE-2016-8579.

Here the reply from CVE Assignment Team:

docker2aci is apparently a library [...] and we almost always recognize
the potential for an unattended use case for any library.
[...]
Someone can call the ConvertSavedFile 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 no human is around to notice
the CPU usage.

Use CVE-2016-8579.

lucab added a commit to lucab/docker2aci that referenced this issue Oct 13, 2016
This commit fixes a possible infinite loop while traversing
the dependency ancestry of a malformed local image file.

This has been assigned CVE-2016-8579:
appc#203 (comment)
@lucab
Copy link
Contributor

lucab commented Oct 13, 2016

@thebeeman a proposed fix for this is up at #204, adding additional validation on crafted images. Can you please take a look at it?

lucab added a commit to lucab/docker2aci that referenced this issue Oct 13, 2016
This commit fixes a possible infinite loop while traversing
the dependency ancestry of a malformed local image file.

This has been assigned CVE-2016-8579:
appc#203 (comment)
lucab added a commit to lucab/docker2aci that referenced this issue Oct 13, 2016
This commit fixes a possible infinite loop while traversing
the dependency ancestry of a malformed local image file.

This has been assigned CVE-2016-8579:
appc#203 (comment)
@soh0ro0t
Copy link
Author

i reviewed the patch for CVE-2016-8579 and processed some tests with the previous malicious image, it addressed the issue.
i agree.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants