Skip to content

Commit

Permalink
+ (Core) Fixed an issue that prevented datetime values from being pas…
Browse files Browse the repository at this point in the history
…sed through the login page URL redirection. (Fixes #5615)
  • Loading branch information
shauncummings committed Nov 28, 2023
1 parent e3e7a21 commit d090d7a
Show file tree
Hide file tree
Showing 5 changed files with 203 additions and 5 deletions.
2 changes: 1 addition & 1 deletion Rock.Blocks/Security/AccountEntry.cs
Expand Up @@ -1082,7 +1082,7 @@ private string GetSafeDecodedUrl( string url )
// Remove the http and https schemes before checking if URL contains XSS objects.
if ( decodedUrl.Replace( "https://", string.Empty )
.Replace( "http://", string.Empty )
.HasXssObjects() )
.RedirectUrlContainsXss() )
{
return null;
}
Expand Down
2 changes: 1 addition & 1 deletion Rock.Blocks/Security/Login.cs
Expand Up @@ -1293,7 +1293,7 @@ private string GetSafeDecodedUrl( string url )
// Remove the http and https schemes before checking if URL contains XSS objects.
if ( decodedUrl.Replace( "https://", string.Empty )
.Replace( "http://", string.Empty )
.HasXssObjects() )
.RedirectUrlContainsXss() )
{
return null;
}
Expand Down
166 changes: 165 additions & 1 deletion Rock.Common/ExtensionMethods/StringExtensions.cs
Expand Up @@ -181,6 +181,8 @@ public static string HmacSha256Hash( this string str, string keyString )
/// Accepts an encoded string and returns an encoded string
/// </summary>
/// <param name="encodedString"></param>
[RockObsolete( "1.16.1" )]
[Obsolete( "This method is no longer suitable. Consider using RedirectUrlContainsXss." )]
public static string ScrubEncodedStringForXSSObjects( this string encodedString )
{
var decodedString = encodedString.GetFullyUrlDecodedValue();
Expand All @@ -200,6 +202,8 @@ public static string ScrubEncodedStringForXSSObjects( this string encodedString
/// <returns>
/// <c>true</c> if <paramref name="decodedString"/> has XSS objects; otherwise, <c>false</c>.
/// </returns>
[RockObsolete( "1.16.1" )]
[Obsolete( "This method is no longer suitable. Consider using RedirectUrlContainsXss." )]
public static bool HasXssObjects( this string decodedString )
{
// Characters used by DOM Objects; javascript, document, window and URLs
Expand All @@ -213,13 +217,65 @@ public static bool HasXssObjects( this string decodedString )
return false;
}

/// <summary>
/// Checks a value intended to be a redirect URL for XSS injection methods.
/// </summary>
/// <param name="redirectUrl">The value intended to be used for a redirect URL.</param>
/// <returns>
/// <c>true</c> if [redirectUrl contains XSS injection methods]; otherwise, <c>false</c>.
/// </returns>
public static bool RedirectUrlContainsXss( this string redirectUrl )
{
if ( string.IsNullOrWhiteSpace( redirectUrl ) )
{
return false; // Early exit, nothing to check.
}

// These are characters which we will not ever allow in a URL query string value. We used to block colon (:), but that
// has a valid use in DateTime values.
char[] badCharacters = new char[] { '<', '>', '*' };

// Ensure we URL and HTML decode all characters, even if they are double/triple/etc encoded.
var decodedString = redirectUrl.GetFullyUrlDecodedValue().GetFullyHtmlDecodedValue();

// Check for bad characters.
if ( decodedString.IndexOfAny( badCharacters ) >= 0 )
{
return true;
}

// Some special whitespace characters are ignored by the browser when parsing script. Remove all whitespace from the
// string and make it lower case for easier comparison.
decodedString = Regex.Replace( decodedString, @"\s+", "" ).ToLower();

// These are strings that should never be allowed in a URL query string value.
string[] blockedStrings = new string[] { "javascript:" };

// Check for bad strings.
foreach ( string blockedString in blockedStrings )
{
if ( decodedString.Contains( blockedString ) )
{
return true;
}
}

// Everything looks okay.
return false;
}

/// <summary>
/// Gets a fully URL-decoded string (or returns string.Empty if it cannot be decoded within 10 attempts).
/// </summary>
/// <param name="encodedString"></param>
/// <param name="encodedString">The encoded string.</param>
/// <returns></returns>
public static string GetFullyUrlDecodedValue( this string encodedString )
{
if ( string.IsNullOrWhiteSpace( encodedString) )
{
return encodedString;
}

int loopCount = 0;
var decodedString = encodedString;
var testString = WebUtility.UrlDecode( encodedString );
Expand All @@ -238,6 +294,114 @@ public static string GetFullyUrlDecodedValue( this string encodedString )
return decodedString;
}

/// <summary>
/// Gets a fully HTML-decoded string (or returns string.Empty if it cannot be decoded within 10 attempts).
/// </summary>
/// <param name="encodedString">The encoded string.</param>
/// <returns></returns>
public static string GetFullyHtmlDecodedValue( this string encodedString )
{
if ( string.IsNullOrWhiteSpace( encodedString ) )
{
return encodedString;
}

int loopCount = 0;
var decodedString = encodedString;
var testString = HtmlDecodeCharactersWithoutSeparators( encodedString );
while ( testString != decodedString )
{
loopCount++;
if ( loopCount >= 10 )
{
return string.Empty;
}

decodedString = testString;
testString = HtmlDecodeCharactersWithoutSeparators( testString );
}

return decodedString;
}

/// <summary>
/// This method inserts missing semi-colon separators into HTML-encoded character strings that use hex or decimal character
/// references before HTML decoding them. Browsers will render these strings without the separators, but
/// <see cref="WebUtility.HtmlDecode"/> will not.
/// </summary>
/// <param name="encodedString">The encoded string.</param>
/// <returns></returns>
private static string HtmlDecodeCharactersWithoutSeparators( this string encodedString )
{
if ( string.IsNullOrWhiteSpace( encodedString ) )
{
return encodedString;
}

// Hex encoded HTML characters should follow the format "&#0000;" (for decimal) or &#x0000; (for hex). If we don't have
// an ampersand together with a pound symbol, we can just use the base method.
if ( !encodedString.Contains( "&#" ) )
{
return WebUtility.HtmlDecode( encodedString );
}

// This loop will shift each segment of the string from encodedString to correctedEncodedString, adding any missing
// semi-colons after each decimal or hex encoded character.
var correctedEncodedString = string.Empty;
while ( encodedString.Contains( "&#" ) )
{
var elementBoundary = encodedString.IndexOf( "&#" );

// Start by putting everything before the next &# into the corrected string and removing it from the original.
correctedEncodedString += encodedString.Substring( 0, elementBoundary ) + "&#";
elementBoundary += 2;
encodedString = encodedString.Substring( elementBoundary, encodedString.Length - elementBoundary );

// If the next character in the string is an "x", move it to the corrected string.
var nextChar = encodedString.FirstOrDefault();
bool useHex = false;
if ( nextChar == 'x' )
{
useHex = true;
correctedEncodedString += nextChar;
encodedString = encodedString.Substring( 1, encodedString.Length - 1 );
nextChar = encodedString.FirstOrDefault();
}

// Keep moving any digits to the corrected string. There's technically no limit on padding zeros.
var isDigit = useHex ? nextChar.IsHexDigit() : char.IsDigit( nextChar );
while ( isDigit )
{
correctedEncodedString += nextChar;
encodedString = encodedString.Substring( 1, encodedString.Length - 1 );
nextChar = encodedString.FirstOrDefault();
isDigit = useHex ? nextChar.IsHexDigit() : char.IsDigit( nextChar );
}

// Add missing semi-colons.
if ( nextChar != ';' )
{
correctedEncodedString += ";";
}
}

// Re-append any trailing characters left over in the original string.
correctedEncodedString += encodedString;

return WebUtility.HtmlDecode( correctedEncodedString );
}

/// <summary>
/// Checks a digit to see if it's a valid hexadecimal character (0-9 or A-F). Permits lowercase.
/// </summary>
/// <param name="c">The character.</param>
/// <returns></returns>
private static bool IsHexDigit( this char c )
{
char[] nonNumericHexCharacters = new char[] { 'A', 'B', 'C', 'D', 'E', 'F', 'a', 'b', 'c', 'd', 'e', 'f' };
return char.IsDigit( c ) || nonNumericHexCharacters.Contains( c );
}

/// <summary>
/// Joins and array of strings using the provided separator.
/// </summary>
Expand Down
Expand Up @@ -234,5 +234,40 @@ public void AsNumeric_NumbersOnly()
}

#endregion

#region RedirectUrlContainsXss

[DataRow( "page/1" )]
[DataRow( "test&nbsp;test" )]
[DataRow( "Occurrence=2023-09-28T09:00:00" )] // Valid date input.
[DataRow( "Occurrence%253d2023-09-28T09%25253a00%25253a00" )] // Valid date input, partially double and triple encoded.
[DataTestMethod]
public void RedirectUrlContainsXss_ValidInput( string input )
{
var output = input.RedirectUrlContainsXss();
Assert.That.AreEqual( output, false );

}

[DataRow( "<style>" )] // Angle brackets.
[DataRow( "%3Cstyle>" )] // URL-encoded Angle brackets.
[DataRow( "&lt;style>" )] // HTML-encoded Angle brackets.
[DataRow( "javas\tcript:alert(0)" )] // Tab character.
[DataRow( "1/+/[*/[]/+alert(1)//" )] // Asterisk character.
[DataRow( "javascript%253Aalert(%27xss%27)" )] // javascript: (with double URL-encoded colon).
[DataRow( "java%0d%0ascript%0d%0a:alert(0)" )] // javascript: (with URL-encoded CR/LF characters).
[DataRow( "javas cript:alert(0)" )] // javascript: (with space character).
// javascript: (HTML-encoded hex character reference).
[DataRow( "&#x6A;&#x61;&#x76;&#x61;&#x73;&#x63;&#x72;&#x69;&#x70;&#x74;&#x3A;" )]
// javascript: (HTML-encoded decimal character reference, no separators).
[DataRow( "&#0000106&#0000097&#0000118&#0000097&#0000115&#0000099&#0000114&#0000105&#0000112&#0000116&#0000058&#0000097&#0000108&#0000101&#0000114&#0000116&#0000040&#0000039&#0000088&#0000083&#0000083&#0000039&#0000041" )]
[DataTestMethod]
public void RedirectUrlContainsXss_RiskyInput( string input )
{
var output = input.RedirectUrlContainsXss();
Assert.That.AreEqual( output, true );
}

#endregion RedirectUrlContainsXss
}
}
3 changes: 1 addition & 2 deletions RockWeb/Blocks/Security/PhoneNumberIdentification.ascx.cs
Expand Up @@ -545,13 +545,12 @@ private void AuthenticatePerson( int personId )
}

var returnUrl = PageParameter( "returnUrl" );
if ( returnUrl.IsNullOrWhiteSpace() )
if ( returnUrl.IsNullOrWhiteSpace() || returnUrl.RedirectUrlContainsXss() )
{
returnUrl = "/";
}
else
{
returnUrl = returnUrl.ScrubEncodedStringForXSSObjects();
returnUrl = Server.UrlDecode( returnUrl );
}

Expand Down

0 comments on commit d090d7a

Please sign in to comment.