Wicket 8.x security fixes

wicket-auth-roles/src/main/java/org/apache/wicket/authroles/authentication/AuthenticatedWebSession.java

@@ -71,6 +71,7 @@ public final boolean signIn(final String username, final String password)
 		}
 		else if (authenticated && signedIn.compareAndSet(false, true))
 		{
+			changeSessionId();
 			bind();
 		}
 		return authenticated;

wicket-core/src/main/java/org/apache/wicket/core/util/string/JavaScriptUtils.java

@@ -95,6 +95,26 @@ public static CharSequence escapeQuotes(final CharSequence input)
 		return s;
 	}
 
+	/**
+	 * Escape single and double quotes so that they can be part of e.g. an alert call.
+	 *
+	 * Note: JSON values need to escape only the double quote, so this method wont help.
+	 *
+	 * @param input
+	 *            the JavaScript which needs to be escaped
+	 * @return Escaped version of the input
+	 */
+	public static CharSequence escapeQuotesAndBackslash(final CharSequence input)
+	{
+		CharSequence s = input;
+		if (s != null)
+		{
+			s = Strings.replaceAll(s, "\\", "\\\\");
+			s = escapeQuotes(s);
+		}
+		return s;
+	}
+
 	/**
 	 * Write a reference to a javascript file to the response object
 	 * 

wicket-core/src/main/java/org/apache/wicket/markup/html/link/ExternalLink.java

@@ -16,6 +16,7 @@
  */
 package org.apache.wicket.markup.html.link;
 
+import org.apache.wicket.core.util.string.JavaScriptUtils;
 import org.apache.wicket.markup.ComponentTag;
 import org.apache.wicket.model.IModel;
 import org.apache.wicket.model.Model;
@@ -193,7 +194,7 @@ else if (getDefaultModel() != null)
 					// generate a popup script by asking popup settings for one
 					if (popupSettings != null)
 					{
-						popupSettings.setTarget("'" + url + "'");
+						popupSettings.setTarget(url);
 						String popupScript = popupSettings.getPopupJavaScript();
 						tag.put("onclick", popupScript);
 					}

wicket-core/src/main/java/org/apache/wicket/markup/html/link/Link.java

@@ -385,7 +385,7 @@ else if (tag.getName().equalsIgnoreCase("script") ||
 				// generate a popup script by asking popup settings for one
 				if (popupSettings != null)
 				{
-					popupSettings.setTarget("'" + url + "'");
+					popupSettings.setTarget(url.toString());
 					String popupScript = popupSettings.getPopupJavaScript();
 					tag.put("onclick", popupScript);
 				}

wicket-core/src/main/java/org/apache/wicket/markup/html/link/PopupSettings.java

@@ -16,6 +16,7 @@
  */
 package org.apache.wicket.markup.html.link;
 
+import org.apache.wicket.core.util.string.JavaScriptUtils;
 import org.apache.wicket.util.io.IClusterable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -155,8 +156,10 @@ public String getPopupJavaScript()
 			windowTitle = windowTitle.replaceAll("\\W", "_");
 		}
 
-	    StringBuilder script = new StringBuilder("var w = window.open(" + target + ", '").append(
-			windowTitle).append("', '");
+		StringBuilder script = new StringBuilder(//
+			"var w = window.open('"//
+				+ JavaScriptUtils.escapeQuotes(target) //
+				+ "', '").append(windowTitle).append("', '");
 
 		script.append("scrollbars=").append(flagToString(SCROLLBARS));
 		script.append(",location=").append(flagToString(LOCATION_BAR));

wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResource.java

@@ -16,17 +16,6 @@
  */
 package org.apache.wicket.request.resource;
 
-import java.io.ByteArrayInputStream;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.Serializable;
-import java.nio.charset.Charset;
-import java.nio.charset.StandardCharsets;
-import java.util.Locale;
-import java.util.Objects;
-
-import javax.servlet.http.HttpServletResponse;
-
 import org.apache.wicket.Application;
 import org.apache.wicket.IWicketInternalException;
 import org.apache.wicket.Session;
@@ -46,7 +35,6 @@
 import org.apache.wicket.util.io.IOUtils;
 import org.apache.wicket.util.lang.Classes;
 import org.apache.wicket.util.lang.Packages;
-import org.apache.wicket.util.resource.IFixedLocationResourceStream;
 import org.apache.wicket.util.resource.IResourceStream;
 import org.apache.wicket.util.resource.ResourceStreamNotFoundException;
 import org.apache.wicket.util.resource.ResourceStreamWrapper;
@@ -55,6 +43,16 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import javax.servlet.http.HttpServletResponse;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.Serializable;
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
+import java.util.Locale;
+import java.util.Objects;
+
 /**
  * Represents a localizable static resource.
  * <p>
@@ -557,38 +555,18 @@ public void setCompress(boolean compress)
 
 	private IResourceStream internalGetResourceStream(final String style, final Locale locale)
 	{
+		if (!accept(absolutePath))
+		{
+			throw new PackageResourceBlockedException(
+				"Access denied to (static) package resource " + absolutePath + ". See IPackageResourceGuard");
+		}
+
 		IResourceStreamLocator resourceStreamLocator = Application.get()
 			.getResourceSettings()
 			.getResourceStreamLocator();
 		IResourceStream resourceStream = resourceStreamLocator.locate(getScope(), absolutePath,
 			style, variation, locale, null, false);
 
-		String realPath = absolutePath;
-		if (resourceStream instanceof IFixedLocationResourceStream)
-		{
-			realPath = ((IFixedLocationResourceStream)resourceStream).locationAsString();
-			if (realPath != null)
-			{
-				int index = realPath.indexOf(absolutePath);
-				if (index != -1)
-				{
-					realPath = realPath.substring(index);
-				}
-			}
-			else
-			{
-				realPath = absolutePath;
-			}
-
-		}
-
-		if (accept(realPath) == false)
-		{
-			throw new PackageResourceBlockedException(
-				"Access denied to (static) package resource " + absolutePath +
-					". See IPackageResourceGuard");
-		}
-
 		if (resourceStream != null)
 		{
 			resourceStream = new ProcessingResourceStream(resourceStream);

wicket-core/src/test/java/org/apache/wicket/core/util/string/JavaScriptUtilsTest.java

@@ -17,9 +17,12 @@
 package org.apache.wicket.core.util.string;
 
 import org.apache.wicket.response.StringResponse;
+import org.hamcrest.CoreMatchers;
 import org.junit.Assert;
 import org.junit.Test;
 
+import static org.hamcrest.CoreMatchers.is;
+
 /**
  * @since 1.5.7
  */
@@ -89,4 +92,9 @@ public void scriptTag()
 			JavaScriptUtils.SCRIPT_OPEN_TAG);
 		assertEquals("\n/*]]>*/\n</script>\n", JavaScriptUtils.SCRIPT_CLOSE_TAG);
 	}
+
+	@Test
+	public void escapeQuotesAndBackslash(){
+		assertThat(JavaScriptUtils.escapeQuotesAndBackslash("alert('foo\\tbar')"), is("alert(\\'foo\\\\tbar\\')"));
+	}
 }

wicket-core/src/test/java/org/apache/wicket/markup/html/PackageResourceTest.java

@@ -16,11 +16,11 @@
  */
 package org.apache.wicket.markup.html;
 
-import java.util.Locale;
-
 import org.apache.wicket.Application;
 import org.apache.wicket.SharedResources;
+import org.apache.wicket.markup.html.snake_case.TestPageInsideSnakeCasePackage;
 import org.apache.wicket.protocol.http.WebApplication;
+import org.apache.wicket.protocol.https.HttpPage;
 import org.apache.wicket.request.resource.JavaScriptPackageResource;
 import org.apache.wicket.request.resource.PackageResource;
 import org.apache.wicket.request.resource.PackageResourceReference;
@@ -30,6 +30,9 @@
 import org.junit.Before;
 import org.junit.Test;
 
+import java.util.Locale;
+
+
 /**
  * Tests for package resources.
  * 
@@ -189,4 +192,55 @@ public void javascriptFileWithEncoding()
 		final String contentType = tester.getLastResponse().getContentType();
 		assertEquals("text/javascript; charset=" + encoding, contentType);
 	}
+
+	@Test
+	public void getResourceStream()
+	{
+		PackageResource resource = new PackageResourceReference(PackageResourceTest.class,
+			"packaged1.txt").getResource();
+		assertNotNull(resource.getResourceStream());
+	}
+
+	@Test(expected = PackageResource.PackageResourceBlockedException.class)
+	public void dontGetResourceStream()
+	{
+		PackageResource resource = new PackageResourceReference(HttpPage.class,
+			"HttpPage.html").getResource();
+		resource.getResourceStream();
+	}
+
+	@Test(expected = PackageResource.PackageResourceBlockedException.class)
+	public void dontGetResourceStreamIfNameHasSuffix()
+	{
+		PackageResource resource = new PackageResourceReference(HttpPage.class,
+			"HttpPage_en.html").getResource();
+		resource.getResourceStream();
+	}
+
+	@Test
+	public void getResourceStreamInSnakeCasePackage()
+	{
+		PackageResource resource = new PackageResourceReference(
+			TestPageInsideSnakeCasePackage.class, "style.css").getResource();
+		assertNotNull(resource.getResourceStream());
+	}
+
+	@Test(expected = PackageResource.PackageResourceBlockedException.class)
+	public void dontGetResourceStreamInSnakeCasePackage()
+	{
+		PackageResource resource = new PackageResourceReference(
+			TestPageInsideSnakeCasePackage.class,
+			"TestPageInsideSnakeCasePackage.html").getResource();
+		resource.getResourceStream();
+	}
+
+	@Test(expected = PackageResource.PackageResourceBlockedException.class)
+	public void dontGetResourceStreamInSnakeCasePackageIfNameHasSuffix()
+	{
+		PackageResource resource = new PackageResourceReference(
+			TestPageInsideSnakeCasePackage.class,
+			"TestPageInsideSnakeCasePackage_en.html").getResource();
+		resource.getResourceStream();
+	}
+
 }

wicket-core/src/test/java/org/apache/wicket/markup/html/link/ClientSideImageMapTest.java

@@ -1,38 +1,38 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.wicket.markup.html.link;
-
-import java.util.Locale;
-
-import org.apache.wicket.util.tester.WicketTestCase;
-import org.junit.Test;
-
-/**
- * @since 1.5
- */
-public class ClientSideImageMapTest extends WicketTestCase
-{
-	/**
-	 * @throws Exception
-	 */
-	@Test
-	public void testRenderClientSideImageMapPage_1() throws Exception
-	{
-		tester.getSession().setLocale(Locale.US);
-		executeTest(ClientSideImageMapPage_1.class, "ClientSideImageMapPageExpectedResult_1.html");
-	}
-}
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.markup.html.link;
+
+import java.util.Locale;
+
+import org.apache.wicket.util.tester.WicketTestCase;
+import org.junit.Test;
+
+/**
+ * @since 1.5
+ */
+public class ClientSideImageMapTest extends WicketTestCase
+{
+	/**
+	 * @throws Exception
+	 */
+	@Test
+	public void testRenderClientSideImageMapPage_1() throws Exception
+	{
+		tester.getSession().setLocale(Locale.US);
+		executeTest(ClientSideImageMapPage_1.class, "ClientSideImageMapPageExpectedResult_1.html");
+	}
+}