KEYCLOAK-17033: Reflected XSS attack with referrer in new account

console
This commit is contained in:
Stan Silvert 2021-02-11 07:35:28 -05:00 committed by Stian Thorgersen
parent 6e3dbfcb3d
commit 3b80eee5bf
2 changed files with 39 additions and 13 deletions

View file

@ -37,6 +37,8 @@ import static org.keycloak.testsuite.util.URLAssert.assertCurrentUrlEquals;
public class ReferrerTest extends AbstractAccountTest { public class ReferrerTest extends AbstractAccountTest {
public static final String FAKE_CLIENT_ID = "fake-client-name"; public static final String FAKE_CLIENT_ID = "fake-client-name";
public static final String REFERRER_LINK_TEXT = "Back to " + LOCALE_CLIENT_NAME_LOCALIZED; public static final String REFERRER_LINK_TEXT = "Back to " + LOCALE_CLIENT_NAME_LOCALIZED;
public static final String FAKE_CLIENT_URL_CONTEXT = "auth/non-existing-page/";
public static final String FAKE_CLIENT_URL_FRAGMENT = "?foo=bar&bar=foo#anchor";
@Page @Page
private WelcomeScreen welcomeScreen; private WelcomeScreen welcomeScreen;
@ -52,19 +54,36 @@ public class ReferrerTest extends AbstractAccountTest {
ClientRepresentation testClient = new ClientRepresentation(); ClientRepresentation testClient = new ClientRepresentation();
testClient.setClientId(FAKE_CLIENT_ID); testClient.setClientId(FAKE_CLIENT_ID);
testClient.setName(LOCALE_CLIENT_NAME); testClient.setName(LOCALE_CLIENT_NAME);
testClient.setRedirectUris(Collections.singletonList(getFakeClientUrl()));
// Redirect URIs are no longer allowed to contain a fragment, so we
// need the wildcard in order to use fragments in tests
testClient.setRedirectUris(Collections.singletonList(getFakeClientUrl("*")));
testClient.setEnabled(true); testClient.setEnabled(true);
testRealm.setClients(Collections.singletonList(testClient)); testRealm.setClients(Collections.singletonList(testClient));
testRealm.setAccountTheme(LOCALIZED_THEME_PREVIEW); // using localized custom theme for the fake client localized name testRealm.setAccountTheme(LOCALIZED_THEME_PREVIEW); // using localized custom theme for the fake client localized name
} }
@Test
// https://issues.redhat.com/browse/KEYCLOAK-17033
// If the referrer is unescaped, this test will throw an exception.
// org.openqa.selenium.UnhandledAlertException: unexpected alert open: {Alert text : XSS}
public void reflectedXSSTest() {
String attackUrl = getFakeClientUrl("'+alert('XSS')+'");
welcomeScreen.navigateTo(FAKE_CLIENT_ID, attackUrl);
welcomeScreen.header().clickLoginBtn();
loginToAccount();
welcomeScreen.clickPersonalInfoLink();
}
@Test @Test
public void loggedInWelcomeScreenTest() { public void loggedInWelcomeScreenTest() {
welcomeScreen.header().clickLoginBtn(); welcomeScreen.header().clickLoginBtn();
loginToAccount(); loginToAccount();
welcomeScreen.navigateTo(FAKE_CLIENT_ID, getFakeClientUrl()); welcomeScreen.navigateTo(FAKE_CLIENT_ID, getFakeClientUrl(FAKE_CLIENT_URL_FRAGMENT));
welcomeScreen.header().assertLoginBtnVisible(false); welcomeScreen.header().assertLoginBtnVisible(false);
welcomeScreen.header().assertLogoutBtnVisible(true); welcomeScreen.header().assertLogoutBtnVisible(true);
@ -73,7 +92,7 @@ public class ReferrerTest extends AbstractAccountTest {
@Test @Test
public void loggedOutWelcomeScreenTest() { public void loggedOutWelcomeScreenTest() {
welcomeScreen.navigateTo(FAKE_CLIENT_ID, getFakeClientUrl()); welcomeScreen.navigateTo(FAKE_CLIENT_ID, getFakeClientUrl(FAKE_CLIENT_URL_FRAGMENT));
welcomeScreen.header().assertLoginBtnVisible(true); welcomeScreen.header().assertLoginBtnVisible(true);
welcomeScreen.header().assertLogoutBtnVisible(false); welcomeScreen.header().assertLogoutBtnVisible(false);
@ -85,7 +104,7 @@ public class ReferrerTest extends AbstractAccountTest {
welcomeScreen.header().clickLoginBtn(); welcomeScreen.header().clickLoginBtn();
loginToAccount(); loginToAccount();
welcomeScreen.navigateTo(FAKE_CLIENT_ID, getFakeClientUrl()); welcomeScreen.navigateTo(FAKE_CLIENT_ID, getFakeClientUrl(FAKE_CLIENT_URL_FRAGMENT));
welcomeScreen.clickPersonalInfoLink(); welcomeScreen.clickPersonalInfoLink();
testReferrer(personalInfoPage.header(), true); testReferrer(personalInfoPage.header(), true);
@ -93,7 +112,7 @@ public class ReferrerTest extends AbstractAccountTest {
@Test @Test
public void loggedOutPageTest() { public void loggedOutPageTest() {
welcomeScreen.navigateTo(FAKE_CLIENT_ID, getFakeClientUrl()); welcomeScreen.navigateTo(FAKE_CLIENT_ID, getFakeClientUrl(FAKE_CLIENT_URL_FRAGMENT));
welcomeScreen.clickPersonalInfoLink(); welcomeScreen.clickPersonalInfoLink();
loginToAccount(); loginToAccount();
@ -102,10 +121,10 @@ public class ReferrerTest extends AbstractAccountTest {
@Test @Test
public void badClientNameTest() { public void badClientNameTest() {
welcomeScreen.navigateTo(FAKE_CLIENT_ID + "-bad", getFakeClientUrl()); welcomeScreen.navigateTo(FAKE_CLIENT_ID + "-bad", getFakeClientUrl(FAKE_CLIENT_URL_FRAGMENT));
testReferrer(welcomeScreen.header(), false); testReferrer(welcomeScreen.header(), false);
welcomeScreen.navigateTo(FAKE_CLIENT_ID + "-bad", getFakeClientUrl()); welcomeScreen.navigateTo(FAKE_CLIENT_ID + "-bad", getFakeClientUrl(FAKE_CLIENT_URL_FRAGMENT));
welcomeScreen.clickPersonalInfoLink(); welcomeScreen.clickPersonalInfoLink();
loginToAccount(); loginToAccount();
testReferrer(personalInfoPage.header(), false); testReferrer(personalInfoPage.header(), false);
@ -113,10 +132,10 @@ public class ReferrerTest extends AbstractAccountTest {
@Test @Test
public void badClientUriTest() { public void badClientUriTest() {
welcomeScreen.navigateTo(FAKE_CLIENT_ID, getFakeClientUrl() + "-bad"); welcomeScreen.navigateTo(FAKE_CLIENT_ID, getFakeClientUrlWithBadContext());
testReferrer(welcomeScreen.header(), false); testReferrer(welcomeScreen.header(), false);
welcomeScreen.navigateTo(FAKE_CLIENT_ID, getFakeClientUrl() + "-bad"); welcomeScreen.navigateTo(FAKE_CLIENT_ID, getFakeClientUrlWithBadContext());
welcomeScreen.clickPersonalInfoLink(); welcomeScreen.clickPersonalInfoLink();
loginToAccount(); loginToAccount();
testReferrer(personalInfoPage.header(), false); testReferrer(personalInfoPage.header(), false);
@ -126,17 +145,24 @@ public class ReferrerTest extends AbstractAccountTest {
if (expectReferrerVisible) { if (expectReferrerVisible) {
assertEquals(REFERRER_LINK_TEXT, header.getReferrerLinkText()); assertEquals(REFERRER_LINK_TEXT, header.getReferrerLinkText());
header.clickReferrerLink(); header.clickReferrerLink();
assertCurrentUrlEquals(getFakeClientUrl()); assertCurrentUrlEquals(getFakeClientUrl(FAKE_CLIENT_URL_FRAGMENT));
} }
else { else {
header.assertReferrerLinkVisible(false); header.assertReferrerLinkVisible(false);
} }
} }
private String getFakeClientUrl() { private String getFakeClientUrl(String suffix) {
// we need to use some page which host exists Firefox is throwing exceptions like crazy if we try to load // we need to use some page which host exists Firefox is throwing exceptions like crazy if we try to load
// a page on a non-existing host, like e.g. http://non-existing-server/ // a page on a non-existing host, like e.g. http://non-existing-server/
// also we need to do this here as getAuthServerRoot is not ready when firing this class' constructor // also we need to do this here as getAuthServerRoot is not ready when firing this class' constructor
return getAuthServerRoot() + "auth/non-existing-page/?foo=bar&bar=foo#anchor"; return getAuthServerRoot() + FAKE_CLIENT_URL_CONTEXT + suffix;
}
private String getFakeClientUrlWithBadContext() {
// we need to use some page which host exists Firefox is throwing exceptions like crazy if we try to load
// a page on a non-existing host, like e.g. http://non-existing-server/
// also we need to do this here as getAuthServerRoot is not ready when firing this class' constructor
return getAuthServerRoot() + "bad/" + FAKE_CLIENT_URL_CONTEXT;
} }
} }

View file

@ -57,7 +57,7 @@
<#if referrer??> <#if referrer??>
var referrer = '${referrer}'; var referrer = '${referrer}';
var referrerName = '${referrerName}'; var referrerName = '${referrerName}';
var referrerUri = '${referrer_uri?no_esc}'; var referrerUri = '${referrer_uri}'.replace('&amp;', '&');
</#if> </#if>
<#if msg??> <#if msg??>