From 0f3cb24604e7c3c1a78d5b97f6f4fce6f796b510 Mon Sep 17 00:00:00 2001 From: James Moger Date: Fri, 29 Mar 2013 10:02:23 -0400 Subject: [PATCH] Enforce security on raw blob page (issue 198) --- releases.moxie | 2 + .../com/gitblit/wicket/pages/BasePage.java | 35 +--------- .../com/gitblit/wicket/pages/RawPage.java | 20 ++++-- .../com/gitblit/wicket/pages/SessionPage.java | 69 +++++++++++++++++++ 4 files changed, 88 insertions(+), 38 deletions(-) create mode 100644 src/main/java/com/gitblit/wicket/pages/SessionPage.java diff --git a/releases.moxie b/releases.moxie index f03af4d7..cd21ab91 100644 --- a/releases.moxie +++ b/releases.moxie @@ -5,6 +5,8 @@ r17: { 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 5c73df33..bb7d8c9e 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.RedirectToUrlException; 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.GitBlitWebSession; 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 @@ public abstract class BasePage extends WebPage { super(); logger = LoggerFactory.getLogger(getClass()); customizeHeader(); - login(); } public BasePage(PageParameters params) { super(params); logger = LoggerFactory.getLogger(getClass()); customizeHeader(); - login(); } private void customizeHeader() { @@ -133,34 +128,6 @@ public abstract class BasePage extends WebPage { 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) { add(new Label("title", getServerName() + " - " + repositoryName)); diff --git a/src/main/java/com/gitblit/wicket/pages/RawPage.java b/src/main/java/com/gitblit/wicket/pages/RawPage.java index 28e8bae2..27a01f96 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 java.util.Map; 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 org.slf4j.LoggerFactory; 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 @@ public class RawPage extends WebPage { 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 00000000..5a255ec5 --- /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(); + } + } +} -- 2.39.5