Inline DTD allows XML bomb attack

[nlevnaut]

This is Wiki page for the vulnerability, it is a very well known XML parser vulnerability: https://en.wikipedia.org/wiki/Billion_laughs_attack.

To replicate this issue in SweetXml, you can do the following in an iex session and watch in the observer:

4 hours later, and it's still running! Memory usage is slowly climbing past 900MB, scheduler 1 utilization hovers 70%.

I was looking into xmerl a bit to see if there's a way to disable inline DTD when using xpath before opening this issue, but I'm not familiar enough with it yet. Hoping someone else may be able to chime in. The closest thing I could find was in the release notes for xmerl 1.2.3 there's an option to turn off external DTD parsing. That sounds like that wouldn't solve this issue though, because internal DTD is the problem.

Maybe something can be done by changing the arguments passed to xmerl_xpath.string/3?

One suggestion to make this safer from @ellispritchard is to set the max heap size for the process calling xmerl functions: http://erlang.org/doc/man/erlang.html#process_flag_max_heap_size

[voltone]

Using 'xmerl_scan' on XML from an untrusted source is generally a bad idea anyway: it creates atoms from XML tag and attribute names, which can bring down the VM by exhausting atom table space.

Anyway, FWIW, it is possible to block DTD entity definitions altogether by using a custom set of rules, and raising on creation of an entity:

  def parse([c | _] = doc, options) when is_integer(c) do
    ets_table = :ets.new(:sweet_xml_rules, [:set, :public])

    try do
      {parsed_doc, _} = :xmerl_scan.string(doc, [no_dtd_entities_rules(ets_table) | options])
      parsed_doc
    after
      :ets.delete(ets_table)
    end
  end

  defp no_dtd_entities_rules(ets_table) do
    {:rules, &no_dtd_entities_rules_read/3, &no_dtd_entities_rules_write/4, ets_table}
  end

  defp no_dtd_entities_rules_write(:entity, _name, _value, _state) do
    raise "DTD entities not allowed"
  end

  defp no_dtd_entities_rules_write(context, name, value, state) do
    ets_table = :xmerl_scan.rules_state(state)

    case :ets.lookup(ets_table, {context, name}) do
      [] ->
        :ets.insert(ets_table, {{context, name}, value})

      _ ->
        :ok
    end

    state
  end

  defp no_dtd_entities_rules_read(context, name, state) do
    ets_table = :xmerl_scan.rules_state(state)

    case :ets.lookup(ets_table, {context, name}) do
      [] ->
        :undefined

      [{_, value}] ->
        value
    end
  end

[axelson]

Are there any mitigations for this?

[ludwikbukowski]

hey, any update on this?

[benbarber]

Has there been any progress on this?

[Shakadak]

Hi benbarber and everyone, we are discussing this internally. We'll come back to you next week.

[Shakadak]

Hi everyone, here are a few vectors that I gathered to help make a decision:

• SweetXml is first and foremost a small DSL intending to translate DOM based data into Elixir's structures, via xpath and its nesting.
  • This means we want to keep it minimal.
  • This means we don't want to do the parsing ourselves.
    • Because of this, we are limited in the scope of what we can or are willing to change.
  • This means the API is not tied to xmerl, altough the implementation currently is.
    • Thanks to this, we could in the future support a different parser, and allow the user to choose wich one to use. But I personally don't know yet how to make the extra dependencies work with the goal of keeping the lib minimal.
• We want to keep backward compatibility as much as possible.
  • This means we won't make the current API restrictive by default.

---

From all that, I'm deciding that:

• We will add options to parse/2, xpath/2, so that the user can restrict the behavior of the parser.
• We will update the doc so that it recommends heavily to set the options for more safety.
• We won't work on the atom exhaustion problem in the near future.
• I personally haven't investiguated stream and stream_tags, but since it is based of :xmerl_scan, the same changes will probably be applied to them.

---

On a last note, it is still possible to open an issue directly for xmerl to bring changes to it, on https://bugs.erlang.org/secure/Dashboard.jspa

[Shakadak]

I did not have much available time to work on it yet, but it's still present in my mind. I'll give an update next week, then try to give updates every two weeks at worst.

[Shakadak]

Hi everyone,

I've made a first implementation in issue-71, but it still need to be documented, real-world tested, and possibly iterated upon before I'm merging this in master

[Shakadak]

Hi everyone, I haven't had time to work on the doc yet, but I've set up a date to do it, I'll update you next week

[Shakadak]

Hi everyone, I'm practically done with the documentation. I haven't published a release candidate, but you can test it with:

{:sweet_xml, github: "kbrw/sweet_xml", tag: "v0.7.0-rc.1"},

I'll get a few teams to test it on our side as well