New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CVE-2019-1000007] xso: fix parser error handling #268

Merged
merged 1 commit into from Jan 12, 2019

Conversation

Projects
None yet
1 participant
@horazont
Copy link
Owner

horazont commented Jan 10, 2019

Needs backport to 0.10!

guard() was incorrectly counting the depth when either of the following was true:

  • the error occured inside the first "start" event on which guard() is used: in that case, guard() would fail to swallow the corresponding "end" event.

  • after an error, further elements appear in the stream before the guard()-ed element is over. in that case, guard() would fail to account for the "start" events caused by those events, and thus let its depth counter go entirely out-of-sync with the XML tree

If this flaw is combined with the use of a supressing xso_error_handler, it is possible to make elements appear higher up in the XML stream tree than they actually are; this implies that it is possible to inject elements in the XML stream.

It requires very specific circumstances for an application to be vulnerable. Example of a vulnerable XSO definition is:

class Baz(aioxmpp.xso.XSO):
    TAG = ("https://xmlns.zombofant.net/aioxmpp/test", "baz")

class Bar(aioxmpp.xso.XSO):
    TAG = ("https://xmlns.zombofant.net/aioxmpp/test", "bar")

    validated = aioxmpp.xso.Attr(
        "foo",
        type_=aioxmpp.xso.JID()
    )

    children = aioxmpp.xso.ChildList([Baz])

@aioxmpp.IQ.as_payload_class
class Foo(aioxmpp.xso.XSO):
    TAG = ("https://xmlns.zombofant.net/aioxmpp/test", "foo")

    child = aioxmpp.xso.Child([Bar])

    def xso_error_handler(self, descriptor, ev_args, exc_info):
        return True

If an attacker sends (formatted for readability):

<iq ... type='result'>
  <foo xmlns='https://xmlns.zombofant.net/aioxmpp/test'>
    <bar foo='&quot;@bar'><baz/><baz/><baz/></bar>
  </foo>
</iq>

to an application which has the above XSO definition loaded, it will see the "end" event of the </iq> on the stream level, breaking the XML stream (because it expects a "start" event instead of an "end" event).

More sophisticated attacks could be used to make an element appear on the stream level instead, which would open the possibility of injecting, for example, <message> stanzas remotely into the stream of a vulnerable aioxmpp client, with arbitrary sender.

xso: fix parser error handling
guard() was incorrectly counting the depth when either of the
following was true:

- the error occured inside the first "start" event on which guard()
  is used: in that case, guard() would fail to swallow the
  corresponding "end" event.

- after an error, further elements appear in the stream before the
  guard()-ed element is over. in that case, guard() would fail to
  account for the "start" events caused by those events, and thus
  let its depth counter go entirely out-of-sync with the XML tree

If this flaw is combined with the use of a supressing
xso_error_handler, it is possible to make elements appear higher
up in the XML stream tree than they actually are; this implies
that it is possible to inject elements in the XML stream.

It requires very specific circumstances for an application to be
vulnerable. Example of a vulnerable XSO definition is:

class Baz(aioxmpp.xso.XSO):
    TAG = ("https://xmlns.zombofant.net/aioxmpp/test", "baz")

class Bar(aioxmpp.xso.XSO):
    TAG = ("https://xmlns.zombofant.net/aioxmpp/test", "bar")

    validated = aioxmpp.xso.Attr(
        "foo",
        type_=aioxmpp.xso.JID()
    )

    children = aioxmpp.xso.ChildList([Baz])

@aioxmpp.IQ.as_payload_class
class Foo(aioxmpp.xso.XSO):
    TAG = ("https://xmlns.zombofant.net/aioxmpp/test", "foo")

    child = aioxmpp.xso.Child([Bar])

    def xso_error_handler(self, descriptor, ev_args, exc_info):
        return True

If an attacker sends:

    <iq ... type='result'><foo xmlns='https://xmlns.zombofant.net/aioxmpp/test'><bar foo='&quot;@bar'><baz/><baz/><baz/></bar></foo></iq>

to an application, it will see the "end" event of the </iq> *on the
stream level*, breaking the XML stream (because it expects a
"start" event instead of an "end" event).

More sophisticated attacks could be used to make an element appear
on the stream level instead, which would open the possibility of
injecting, for example, <message> stanzas remotely into the stream
of a vulnerable aioxmpp client, with arbitrary sender.

@horazont horazont requested a review from sebastianriese Jan 10, 2019

@horazont horazont added the bug label Jan 10, 2019

@horazont horazont added this to the v0.11 milestone Jan 10, 2019

@horazont horazont self-assigned this Jan 10, 2019

@horazont

This comment has been minimized.

Copy link
Owner Author

horazont commented Jan 10, 2019

Backport prepared as f151f92.

@horazont

This comment has been minimized.

Copy link
Owner Author

horazont commented Jan 12, 2019

Backport released in v0.10.3.

@horazont horazont merged commit 29ff083 into devel Jan 12, 2019

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.0002%) to 97.928%
Details

@iamleot iamleot referenced this pull request Jan 23, 2019

Merged

more cves #1485

@horazont

This comment has been minimized.

Copy link
Owner Author

horazont commented Jan 23, 2019

This has been assigned CVE-2019-1000007.

@horazont horazont changed the title xso: fix parser error handling [CVE-2019-1000007] xso: fix parser error handling Jan 23, 2019

@horazont horazont deleted the feature/fix-guard branch Jan 23, 2019

@horazont horazont restored the feature/fix-guard branch Jan 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment