Infinite loop calling to_json on ActiveModel::Name

[skorth]

Do we add an advisory on this?

http://osvdb.org/show/osvdb/118954 https://github.com/rails/rails/pull/19055 https://github.com/rails/rails/issues/19050

[skorth]

corresponding oss-security entry http://www.openwall.com/lists/oss-security/2015/03/06/7

[postmodern]

Yep, it's a DoS vulnerability.

[matthewd]

Yep, it's a DoS vulnerability.

Is it?

At a first look, I don't see the angle... but if there is an actual danger (read: a means by which an attacker could inject an AM::Name into code that doesn't normally see one, and thus might call to_json), I'd love to hear about it: security@rubyonrails.org

[jeremyolliver]

I think the possibility of DDoS is just assumed due to the presence of an infinite looping behaviour - rather than any particular method to coerce an app to actually trigger the bug.

[jasnow]

https://cxsecurity.com/issue/WLB-2015030039 has the same title Ruby on Rails ActiveModel::Name to_json Call Infinite Loop Remote DoS as this issue.

[jasnow]

Also found these references:

• https://github.com/rails/rails/pull/19055
• https://seclists.org/oss-sec/2015/q1/770
• https://seclists.org/oss-sec/2015/q1/791

[jasnow]

After reaching out to the Rails Security team last night, I quickly received this response:

Thank you for reaching out.
At the moment we don’t consider this a security issue, so no CVE should be created for it. 

What make you believe this is a security issue?

[postmodern]

I still think this could be used as a DoS. An attacker could send a request to trigger the infinite .to_json loop which would cause the request to use 100% CPU until it timed out. The attacker could write a script to send a bunch of these requests and overload the web server.

[postmodern]

OTOH, this infinite loop is reproducible every time. So it's unlikely an attacker could somehow trigger this, but it somehow would not trigger under normal circumstances.

[matthewd]

I still don't understand how this is anything more than a bug.

bunch of these requests

@postmodern what requests? If you have an endpoint that evals user input, DoS is the least of your concerns. If you don't... how is an attacker going to invoke this method?

The fact that Kernel#loop doesn't have a CVE on it shows that it takes more than the existence of a possibility that application code could be devised in a way that creates an infinite loop... what qualifies this method as being security-relevant?

[postmodern]

@matthewd I was trying to imagine a scenario where a specific route accepted params to control what data to include into a JSON response or possibly the value to call .to_json on, and somehow an attacker could trick the app into calling .to_json on TheModel.name? This wouldn't require evaling arbitrary input. Although, I think that scenario is pretty unlikely.

What would make this a DoS vuln is if an attacker could somehow trigger the infinite loop with a specially crafted request, but normally the loop wouldn't be triggered. The attacker could then repeatedly hammer the web server with such requests, causing the app to consume CPU and degrade performance. This is similar to how the Slowloris vulnerability or how the famous LOIC DDoS tool worked.

[matthewd]

What would make this a DoS vuln is if an attacker could somehow trigger the infinite loop with a specially crafted request

100% agree on this. An attacker-triggerable infinite loop is a DoS by definition.

We just haven't advanced past "if application code calls an infinite loop, then an attacker could trigger an infinite loop", which is a tautology, not a framework vulnerability.

I don't see how you trick an application into getting an ActiveModel::Name instance in an unexpected place without an eval, or something morally equivalent.

Again, there are a whole raft of methods on a loop Enumerator that will iloop when called (including to_json, when Active Support is loaded). If somehow an attacker tricked an app into calling to_a on such an Enumerator, that would be a DoS. If somehow an attacker tricked an app into calling delete_all on User.all, that would be more than a DoS. We can only ask "somehow" to bear so much weight.

You're the security expert here, not me, but I still feel pretty strongly about this:

The existence of a method does not a framework vulnerability make: it needs to be exploitable[^1] in some way for it to be a danger. Without a theoretical means of invoking it, this is at worst a bug, and is incidentally one of many ways an attacker could "escalate" some other RCE into a DoS.

If the framework creates a vulnerability in an application only when it has an unreasonable combination of configuration options, that's still a framework vulnerability. But it's not a framework vulnerability that it's possible for an application to define a route that does User.all.update(admin: true).

[^1]: theorycraft is fine, I'm not saying you need an implemented exploit... but "this method exists; maybe an application could be tricked into invoking it" is not that.

[postmodern]

We just haven't advanced past "if application code calls an infinite loop, then an attacker could trigger an infinite loop", which is a tautology, not a framework vulnerability.

The problem is ActiveModel::Name.to_json is not supposed to trigger an infinite loop, unlike say loop do. This is unexpected behavior from the developers stand point.

I don't see how you trick an application into getting an ActiveModel::Name instance in an unexpected place without an eval, or something morally equivalent.

You could have an instance method on the model which returns ActiveModel::Name or self.class.model_name? Maybe combined with a .send() which accepts a params value? Maybe some sort of formatting Hash lookup table that also includes .model_name? Or maybe a simple if/else?

def query_json
  value = if params[:field] == 'name'
    MyModel.model_name
  elsif params[:id]
    MyModel.find(params[:id])
  end

  render json: value
end

Again, there are a whole raft of methods on a loop Enumerator that will iloop when called

The problem with this analogy is that loop is supposed to infinitely loop. ActiveModel::Name.to_json is not supposed to infinitely loop. If I call loop do ... than I expect it to infinitely loop. If I call Activemodel::Name.to_json than I expect it to return JSON, not infinitely loop. If an attacker can trigger an infinite loop and DoS my application, when I expected it to return JSON, that is bad.

If somehow an attacker tricked an app into calling delete_all on User.all, that would be more than a DoS.

That is a destructive operation and would only run once. Subsequent User.delete_all would return instantly because there would be no users left to delete. Also, many of the Enumerable methods can detect cyclical references.

The existence of a method does not a framework vulnerability make: it needs to be exploitable.

And what if to exploit the vulnerable method, the developer needs to allow calling the method in some sort of exotic scenario or configuration? A vulnerability does not have to be 100% automatically remotely exploitable in order to be considered a vulnerability. Vulnerabilities often require certain user configurations or certain usages. This is why many DoS vulnerabilities are marked as "potential DoS" or include the phrase "under certain circumstances".

Without a theoretical means of invoking it, this is at worst a bug, and is incidentally one of many ways an attacker could "escalate" some other RCE into a DoS.

See the above example which does not require RCE to trigger. While it may be contrived, it's certainly plausible. I have seen stranger database ORM formatting code.

[matthewd]

While it may be contrived, it's certainly plausible

It's contrived to literally call a method that always infinitely loops [up to the affected versions]. That's not a tricked app, that's a broken app. It's not the kernel's fault for carry the network packet, and it's not ruby's fault for implementing recursion. It is the framework's fault for having a method that when called will always get stuck -- but it's not a vulnerability until the application goes out of its way to expose the ability to call that method.

The fact that File.unlink will delete anything specified doesn't become a vulnerability when an application calls File.unlink(params[:name]), even if the application author thought that method would only operate on files within the cwd.

This is unexpected behavior from the developers stand point.

If it is your position that any bug is by definition a vulnerability (because if an app is "tricked" into calling it, that will result in different behaviour than a developer theoretically expected), then I will agree that this bug fell into that bucket.

Subsequent User.delete_all would return instantly because there would be no users left to delete

Yes, your CPU will be safe... but I think your users might find their service is still denied thereafter. But my point was just that in discussing potential vulnerabilities, I think we need to put some effort into identifying a possible definition of the "somehow", not merely hand-wave it away.

And what if to exploit the vulnerable method, the developer needs to allow calling the method in some sort of exotic scenario or configuration?

Exotic, yes. That's what I said here:

If the framework creates a vulnerability in an application only when it has an unreasonable combination of configuration options, that's still a framework vulnerability.

We have a history of security releases that describe complex multi-prereq scenarios for an application to be vulnerable, backing that up.

But exotic is not artificial. An application could throw a model name into to_json, just as an application could throw an iloop into to_a. Doing so is a bug, creating an immediately broken application, not a subtle flaw waiting to be exploited by a savvy attacker. Wrapping the buggy code in if params[:secret] == OBSCURE_VALUE doesn't change that.

There is a world of difference between "using the framework in an unusual way", and "writing independently exploitable code in the vicinity of the framework".

If every possible broken-by-design application is a framework vulnerability, then I think you eliminate the communication value in enumerating vulnerabilities at all.

Maybe combined with a .send() which accepts a params value?

That's an example of something that's morally an eval, and is an RCE. Please don't do that. :smile:

[postmodern]

Asked the great and wise Kurt Seifried, and he states this does not warrant a CVE, but an open source Rails application which managed to trigger the bug would get it's own CVE. So closing this. https://infosec.exchange/@kurtseifried/110555755579616817