From 0f3cb24604e7c3c1a78d5b97f6f4fce6f796b510 Mon Sep 17 00:00:00 2001
From: James Moger <james.moger@gitblit.com>
Date: Fri, 29 Mar 2013 10:02:23 -0400
Subject: [PATCH] Enforce security on raw blob page (issue 198)
---
src/main/java/com/gitblit/wicket/pages/BasePage.java | 35 -----------
src/main/java/com/gitblit/wicket/pages/RawPage.java | 20 +++++-
src/main/java/com/gitblit/wicket/pages/SessionPage.java | 69 +++++++++++++++++++++++
releases.moxie | 2
4 files changed, 88 insertions(+), 38 deletions(-)
diff --git a/releases.moxie b/releases.moxie
index f03af4d..cd21ab9 100644
--- a/releases.moxie
+++ b/releases.moxie
@@ -5,6 +5,8 @@
title: Gitblit ${project.version} Released
id: ${project.version}
date: ${project.buildDate}
+ security:
+ - Raw servlet was insecure. If someone knew the exact repository name and path to a file, the raw blob could be retrieved bypassing security constraints. (issue 198)
fixes:
- Could not reset settings with $ or { characters through Gitblit Manager because they are not properly escaped
- Fix NPE when getting user's fork without repository list caching (issue 182)
diff --git a/src/main/java/com/gitblit/wicket/pages/BasePage.java b/src/main/java/com/gitblit/wicket/pages/BasePage.java
index 5c73df3..bb7d8c9 100644
--- a/src/main/java/com/gitblit/wicket/pages/BasePage.java
+++ b/src/main/java/com/gitblit/wicket/pages/BasePage.java
@@ -38,15 +38,12 @@
import org.apache.wicket.RequestCycle;
import org.apache.wicket.RestartResponseException;
import org.apache.wicket.markup.html.CSSPackageResource;
-import org.apache.wicket.markup.html.WebPage;
import org.apache.wicket.markup.html.basic.Label;
import org.apache.wicket.markup.html.link.BookmarkablePageLink;
import org.apache.wicket.markup.html.link.ExternalLink;
import org.apache.wicket.markup.html.panel.FeedbackPanel;
import org.apache.wicket.markup.html.panel.Fragment;
import org.apache.wicket.protocol.http.RequestUtils;
-import org.apache.wicket.protocol.http.WebRequest;
-import org.apache.wicket.protocol.http.WebResponse;
import org.apache.wicket.protocol.http.servlet.ServletWebRequest;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -68,7 +65,7 @@
import com.gitblit.wicket.WicketUtils;
import com.gitblit.wicket.panels.LinkPanel;
-public abstract class BasePage extends WebPage {
+public abstract class BasePage extends SessionPage {
private final Logger logger;
@@ -78,14 +75,12 @@
super();
logger = LoggerFactory.getLogger(getClass());
customizeHeader();
- login();
}
public BasePage(PageParameters params) {
super(params);
logger = LoggerFactory.getLogger(getClass());
customizeHeader();
- login();
}
private void customizeHeader() {
@@ -132,34 +127,6 @@
}
super.onAfterRender();
}
-
- private void login() {
- GitBlitWebSession session = GitBlitWebSession.get();
- if (session.isLoggedIn() && !session.isSessionInvalidated()) {
- // already have a session, refresh usermodel to pick up
- // any changes to permissions or roles (issue-186)
- UserModel user = GitBlit.self().getUserModel(session.getUser().username);
- session.setUser(user);
- return;
- }
-
- // try to authenticate by servlet request
- HttpServletRequest httpRequest = ((WebRequest) getRequestCycle().getRequest()).getHttpServletRequest();
- UserModel user = GitBlit.self().authenticate(httpRequest);
-
- // Login the user
- if (user != null) {
- // issue 62: fix session fixation vulnerability
- session.replaceSession();
- session.setUser(user);
-
- // Set Cookie
- WebResponse response = (WebResponse) getRequestCycle().getResponse();
- GitBlit.self().setCookie(response, user);
-
- session.continueRequest();
- }
- }
protected void setupPage(String repositoryName, String pageName) {
if (repositoryName != null && repositoryName.trim().length() > 0) {
diff --git a/src/main/java/com/gitblit/wicket/pages/RawPage.java b/src/main/java/com/gitblit/wicket/pages/RawPage.java
index 28e8bae..27a01f9 100644
--- a/src/main/java/com/gitblit/wicket/pages/RawPage.java
+++ b/src/main/java/com/gitblit/wicket/pages/RawPage.java
@@ -22,7 +22,6 @@
import org.apache.wicket.IRequestTarget;
import org.apache.wicket.PageParameters;
import org.apache.wicket.RequestCycle;
-import org.apache.wicket.markup.html.WebPage;
import org.apache.wicket.protocol.http.WebResponse;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
@@ -31,17 +30,20 @@
import com.gitblit.GitBlit;
import com.gitblit.Keys;
+import com.gitblit.models.RepositoryModel;
+import com.gitblit.models.UserModel;
import com.gitblit.utils.JGitUtils;
import com.gitblit.utils.StringUtils;
+import com.gitblit.wicket.GitBlitWebSession;
import com.gitblit.wicket.WicketUtils;
-public class RawPage extends WebPage {
+public class RawPage extends SessionPage {
private final Logger logger = LoggerFactory.getLogger(getClass().getSimpleName());
public RawPage(final PageParameters params) {
super(params);
-
+
if (!params.containsKey("r")) {
error(getString("gb.repositoryNotSpecified"));
redirectToInterceptPage(new RepositoriesPage());
@@ -60,7 +62,17 @@
final String objectId = WicketUtils.getObject(params);
final String blobPath = WicketUtils.getPath(params);
String[] encodings = GitBlit.getEncodings();
-
+ GitBlitWebSession session = GitBlitWebSession.get();
+ UserModel user = session.getUser();
+
+ RepositoryModel model = GitBlit.self().getRepositoryModel(user, repositoryName);
+ if (model == null) {
+ // user does not have permission
+ error(getString("gb.canNotLoadRepository") + " " + repositoryName);
+ redirectToInterceptPage(new RepositoriesPage());
+ return;
+ }
+
Repository r = GitBlit.self().getRepository(repositoryName);
if (r == null) {
error(getString("gb.canNotLoadRepository") + " " + repositoryName);
diff --git a/src/main/java/com/gitblit/wicket/pages/SessionPage.java b/src/main/java/com/gitblit/wicket/pages/SessionPage.java
new file mode 100644
index 0000000..5a255ec
--- /dev/null
+++ b/src/main/java/com/gitblit/wicket/pages/SessionPage.java
@@ -0,0 +1,69 @@
+/*
+ * Copyright 2013 gitblit.com.
+ *
+ * Licensed 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 com.gitblit.wicket.pages;
+
+import javax.servlet.http.HttpServletRequest;
+
+import org.apache.wicket.PageParameters;
+import org.apache.wicket.markup.html.WebPage;
+import org.apache.wicket.protocol.http.WebRequest;
+import org.apache.wicket.protocol.http.WebResponse;
+
+import com.gitblit.GitBlit;
+import com.gitblit.models.UserModel;
+import com.gitblit.wicket.GitBlitWebSession;
+
+public abstract class SessionPage extends WebPage {
+
+ public SessionPage() {
+ super();
+ login();
+ }
+
+ public SessionPage(final PageParameters params) {
+ super(params);
+ login();
+ }
+
+ private void login() {
+ GitBlitWebSession session = GitBlitWebSession.get();
+ if (session.isLoggedIn() && !session.isSessionInvalidated()) {
+ // already have a session, refresh usermodel to pick up
+ // any changes to permissions or roles (issue-186)
+ UserModel user = GitBlit.self().getUserModel(session.getUser().username);
+ session.setUser(user);
+ return;
+ }
+
+ // try to authenticate by servlet request
+ HttpServletRequest httpRequest = ((WebRequest) getRequestCycle().getRequest())
+ .getHttpServletRequest();
+ UserModel user = GitBlit.self().authenticate(httpRequest);
+
+ // Login the user
+ if (user != null) {
+ // issue 62: fix session fixation vulnerability
+ session.replaceSession();
+ session.setUser(user);
+
+ // Set Cookie
+ WebResponse response = (WebResponse) getRequestCycle().getResponse();
+ GitBlit.self().setCookie(response, user);
+
+ session.continueRequest();
+ }
+ }
+}
--
Gitblit v1.9.1