Skip to content

Commit dea4f52

Browse files
committed
Fix inconsistent results with using regex matches in decode_raw_saml
After running SAML code in production against a Norwegian IdP in a similar library , it was found by Jørgen Fjeld <jorgen at veridit.no> that it is unreliable to check for /^</. This is because the ^ matches any line and not just the first one. It does not help replacing it with \A (Which is the beginning of the string) as the zlib compressed data may start with a "<". The Solution implemented in this patch is to check that the current string is Base64 complient. Because XML can't be Base64 compliant, we check if the string is base64 encoded and if it isn't we return the string with the assumption that it is the XML. If it is we try and decode + inflate. This make the function idempotent and decode_raw_saml(decode_raw_saml(Response)) will yield the correct result.
1 parent c8deef8 commit dea4f52

File tree

1 file changed

+24
-8
lines changed

1 file changed

+24
-8
lines changed

lib/onelogin/ruby-saml/saml_message.rb

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,16 +37,23 @@ def validation_error(message)
3737

3838
private
3939

40+
##
41+
# Take a SAML object provided by +saml+, determine its status and return
42+
# a decoded XML as a String.
43+
#
44+
# Since SAML decided to use the RFC1951 and therefor has no zlib markers,
45+
# the only reliable method of deciding whether we have a zlib stream or not
46+
# is to try and inflate it and fall back to the base64 decoded string if
47+
# the stream contains errors.
4048
def decode_raw_saml(saml)
41-
if saml =~ /^</
42-
return saml
43-
elsif (decoded = decode(saml)) =~ /^</
44-
return decoded
45-
elsif (inflated = inflate(decoded)) =~ /^</
46-
return inflated
47-
end
49+
return saml unless is_base64?(saml)
4850

49-
return nil
51+
decoded = decode(saml)
52+
begin
53+
inflate(decoded)
54+
rescue
55+
decoded
56+
end
5057
end
5158

5259
def encode_raw_saml(saml, settings)
@@ -63,6 +70,15 @@ def encode(encoded)
6370
Base64.encode64(encoded).gsub(/\n/, "")
6471
end
6572

73+
##
74+
# Check if +string+ is base64 encoded
75+
#
76+
# The function is not strict and does allow newline. This is because some SAML implementations
77+
# uses newline in the base64-encoded data, even if they shouldn't have (RFC4648).
78+
def is_base64?(string)
79+
string.match(%r{\A(([A-Za-z0-9+/]{4})|\n)*([A-Za-z0-9+/]{4}|[A-Za-z0-9+/]{3}=|[A-Za-z0-9+/]{2}==)\Z})
80+
end
81+
6682
def escape(unescaped)
6783
CGI.escape(unescaped)
6884
end

0 commit comments

Comments
 (0)