Skip to content
Permalink
Browse files Browse the repository at this point in the history
Fix security vulnerability related to Git pre/post receive callback
  • Loading branch information
robinshine committed May 30, 2022
1 parent 910bb99 commit f1e9768
Show file tree
Hide file tree
Showing 7 changed files with 17 additions and 20 deletions.
Expand Up @@ -537,7 +537,7 @@ private boolean isGitHookValid(File gitDir, String hookName) {

try {
String content = FileUtils.readFileToString(hookFile, Charset.defaultCharset());
if (!content.contains("ENV_GIT_ALTERNATE_OBJECT_DIRECTORIES"))
if (!content.contains("ONEDEV_HOOK_TOKEN"))
return false;
} catch (IOException e) {
throw new RuntimeException(e);
Expand Down
Expand Up @@ -143,6 +143,7 @@ protected void processPacks(final HttpServletRequest request, final HttpServletR
environments.put("ONEDEV_CURL", settingManager.getSystemSetting().getCurlConfig().getExecutable());
environments.put("ONEDEV_URL", serverUrl);
environments.put("ONEDEV_USER_ID", SecurityUtils.getUserId().toString());
environments.put("ONEDEV_HOOK_TOKEN", GitUtils.HOOK_TOKEN);
environments.put("ONEDEV_REPOSITORY_ID", project.getId().toString());

// to be compatible with old repository
Expand Down
Expand Up @@ -75,6 +75,7 @@ private Map<String, String> buildGitEnvs(Project project) {
SettingManager settingManager = OneDev.getInstance(SettingManager.class);
environments.put("ONEDEV_CURL", settingManager.getSystemSetting().getCurlConfig().getExecutable());
environments.put("ONEDEV_URL", serverUrl);
environments.put("ONEDEV_HOOK_TOKEN", GitUtils.HOOK_TOKEN);
environments.put("ONEDEV_USER_ID", SecurityUtils.getUserId().toString());
environments.put("ONEDEV_REPOSITORY_ID", project.getId().toString());
return environments;
Expand Down
3 changes: 3 additions & 0 deletions server-core/src/main/java/io/onedev/server/git/GitUtils.java
Expand Up @@ -16,6 +16,7 @@

import javax.annotation.Nullable;

import org.apache.commons.lang3.RandomStringUtils;
import org.bouncycastle.bcpg.ArmoredOutputStream;
import org.bouncycastle.bcpg.BCPGOutputStream;
import org.bouncycastle.bcpg.HashAlgorithmTags;
Expand Down Expand Up @@ -97,6 +98,8 @@ public class GitUtils {

private static final Logger logger = LoggerFactory.getLogger(GitUtils.class);

public static final String HOOK_TOKEN = RandomStringUtils.randomAlphanumeric(20);

public static boolean isEmptyPath(String path) {
return Strings.isNullOrEmpty(path) || Objects.equal(path, DiffEntry.DEV_NULL);
}
Expand Down
@@ -1,7 +1,6 @@
package io.onedev.server.git.hookcallback;

import java.io.IOException;
import java.net.InetAddress;
import java.util.ArrayList;
import java.util.Enumeration;
import java.util.List;
Expand Down Expand Up @@ -56,17 +55,14 @@ public GitPostReceiveCallback(ProjectManager projectManager, SessionManager sess
@Sessional
@Override
protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
String clientIp = request.getHeader("X-Forwarded-For");
if (clientIp == null) clientIp = request.getRemoteAddr();

if (!InetAddress.getByName(clientIp).isLoopbackAddress()) {
List<String> fields = StringUtils.splitAndTrim(request.getPathInfo(), "/");
Preconditions.checkState(fields.size() == 3);
if (!fields.get(2).equals(GitUtils.HOOK_TOKEN)) {
response.sendError(HttpServletResponse.SC_FORBIDDEN,
"Git hook callbacks can only be accessed from localhost.");
"Git hook callbacks can only be accessed by OneDev itself");
return;
}

List<String> fields = StringUtils.splitAndTrim(request.getPathInfo(), "/");
Preconditions.checkState(fields.size() == 2);

Project project = projectManager.load(Long.valueOf(fields.get(0)));
Long userId = Long.valueOf(fields.get(1));
Expand Down
@@ -1,7 +1,6 @@
package io.onedev.server.git.hookcallback;

import java.io.IOException;
import java.net.InetAddress;
import java.util.ArrayList;
import java.util.Enumeration;
import java.util.HashMap;
Expand Down Expand Up @@ -71,18 +70,15 @@ private void error(Output output, @Nullable String refName, List<String> message
@Sessional
@Override
protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
String clientIp = request.getHeader("X-Forwarded-For");
if (clientIp == null) clientIp = request.getRemoteAddr();

if (!InetAddress.getByName(clientIp).isLoopbackAddress()) {
List<String> fields = StringUtils.splitAndTrim(request.getPathInfo(), "/");
Preconditions.checkState(fields.size() == 3);
if (!fields.get(2).equals(GitUtils.HOOK_TOKEN)) {
response.sendError(HttpServletResponse.SC_FORBIDDEN,
"Git hook callbacks can only be accessed from localhost.");
"Git hook callbacks can only be accessed by OneDev itself");
return;
}

List<String> fields = StringUtils.splitAndTrim(request.getPathInfo(), "/");
Preconditions.checkState(fields.size() == 2);

SecurityUtils.getSubject().runAs(SecurityUtils.asPrincipal(Long.valueOf(fields.get(1))));
try {
Project project = projectManager.load(Long.valueOf(fields.get(0)));
Expand Down
2 changes: 1 addition & 1 deletion server-core/src/main/resources/git-receive-hook
Expand Up @@ -2,7 +2,7 @@
unset http_proxy
unset https_proxy
IFS=$'\r\n';
lines=($(${ONEDEV_CURL} -k -s -S -f -X POST --data-urlencode "ENV_GIT_ALTERNATE_OBJECT_DIRECTORIES=${GIT_ALTERNATE_OBJECT_DIRECTORIES}" --data-urlencode "ENV_GIT_OBJECT_DIRECTORY=${GIT_OBJECT_DIRECTORY}" --data-urlencode "ENV_GIT_QUARANTINE_PATH=${GIT_QUARANTINE_PATH}" -d @- ${ONEDEV_URL}/%s/${ONEDEV_REPOSITORY_ID}/${ONEDEV_USER_ID} 2>&1))
lines=($(${ONEDEV_CURL} -k -s -S -f -X POST --data-urlencode "ENV_GIT_ALTERNATE_OBJECT_DIRECTORIES=${GIT_ALTERNATE_OBJECT_DIRECTORIES}" --data-urlencode "ENV_GIT_OBJECT_DIRECTORY=${GIT_OBJECT_DIRECTORY}" --data-urlencode "ENV_GIT_QUARANTINE_PATH=${GIT_QUARANTINE_PATH}" -d @- ${ONEDEV_URL}/%s/${ONEDEV_REPOSITORY_ID}/${ONEDEV_USER_ID}/${ONEDEV_HOOK_TOKEN} 2>&1))

returnCode=0;

Expand Down

0 comments on commit f1e9768

Please sign in to comment.