From fc3a39d464b1303f0b7d01d0160f81cbbb80a98b Mon Sep 17 00:00:00 2001 From: James Moger <james.moger@gitblit.com> Date: Sun, 07 Sep 2014 11:42:40 -0400 Subject: [PATCH] Create infrastructure for XSS sanitization --- src/main/java/com/gitblit/manager/RepositoryManager.java | 114 ++++++++++++++++++++++++++++++++++++++++++++------------ 1 files changed, 89 insertions(+), 25 deletions(-) diff --git a/src/main/java/com/gitblit/manager/RepositoryManager.java b/src/main/java/com/gitblit/manager/RepositoryManager.java index e0721c7..a8b2323 100644 --- a/src/main/java/com/gitblit/manager/RepositoryManager.java +++ b/src/main/java/com/gitblit/manager/RepositoryManager.java @@ -21,6 +21,7 @@ import java.lang.reflect.Field; import java.net.URI; import java.net.URISyntaxException; +import java.nio.charset.Charset; import java.text.MessageFormat; import java.text.SimpleDateFormat; import java.util.ArrayList; @@ -52,6 +53,7 @@ import org.eclipse.jgit.storage.file.WindowCacheConfig; import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.FileUtils; +import org.eclipse.jgit.util.RawParseUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -422,11 +424,12 @@ @Override public void addToCachedRepositoryList(RepositoryModel model) { if (settings.getBoolean(Keys.git.cacheRepositoryList, true)) { - repositoryListCache.put(model.name.toLowerCase(), model); + String key = getRepositoryKey(model.name); + repositoryListCache.put(key, model); // update the fork origin repository with this repository clone if (!StringUtils.isEmpty(model.originRepository)) { - String originKey = model.originRepository.toLowerCase(); + String originKey = getRepositoryKey(model.originRepository); if (repositoryListCache.containsKey(originKey)) { RepositoryModel origin = repositoryListCache.get(originKey); origin.addFork(model.name); @@ -445,7 +448,8 @@ if (StringUtils.isEmpty(name)) { return null; } - return repositoryListCache.remove(name.toLowerCase()); + String key = getRepositoryKey(name); + return repositoryListCache.remove(key); } /** @@ -554,7 +558,7 @@ // rebuild fork networks for (RepositoryModel model : repositoryListCache.values()) { if (!StringUtils.isEmpty(model.originRepository)) { - String originKey = model.originRepository.toLowerCase(); + String originKey = getRepositoryKey(model.originRepository); if (repositoryListCache.containsKey(originKey)) { RepositoryModel origin = repositoryListCache.get(originKey); origin.addFork(model.name); @@ -590,15 +594,13 @@ /** * Returns the JGit repository for the specified name. * - * @param repositoryName + * @param name * @param logError * @return repository or null */ @Override - public Repository getRepository(String repositoryName, boolean logError) { - // Decode url-encoded repository name (issue-278) - // http://stackoverflow.com/questions/17183110 - repositoryName = repositoryName.replace("%7E", "~").replace("%7e", "~"); + public Repository getRepository(String name, boolean logError) { + String repositoryName = fixRepositoryName(name); if (isCollectingGarbage(repositoryName)) { logger.warn(MessageFormat.format("Rejecting request for {0}, busy collecting garbage!", repositoryName)); @@ -680,16 +682,14 @@ * Returns the repository model for the specified repository. This method * does not consider user access permissions. * - * @param repositoryName + * @param name * @return repository model or null */ @Override - public RepositoryModel getRepositoryModel(String repositoryName) { - // Decode url-encoded repository name (issue-278) - // http://stackoverflow.com/questions/17183110 - repositoryName = repositoryName.replace("%7E", "~").replace("%7e", "~"); + public RepositoryModel getRepositoryModel(String name) { + String repositoryName = fixRepositoryName(name); - String repositoryKey = repositoryName.toLowerCase(); + String repositoryKey = getRepositoryKey(repositoryName); if (!repositoryListCache.containsKey(repositoryKey)) { RepositoryModel model = loadRepositoryModel(repositoryName); if (model == null) { @@ -702,7 +702,7 @@ // cached model RepositoryModel model = repositoryListCache.get(repositoryKey); - if (gcExecutor.isCollectingGarbage(model.name)) { + if (isCollectingGarbage(model.name)) { // Gitblit is busy collecting garbage, use our cached model RepositoryModel rm = DeepCopier.copy(model); rm.isCollectingGarbage = true; @@ -755,6 +755,52 @@ } } return count; + } + + /** + * Replaces illegal character patterns in a repository name. + * + * @param repositoryName + * @return a corrected name + */ + private String fixRepositoryName(String repositoryName) { + if (StringUtils.isEmpty(repositoryName)) { + return repositoryName; + } + + // Decode url-encoded repository name (issue-278) + // http://stackoverflow.com/questions/17183110 + String name = repositoryName.replace("%7E", "~").replace("%7e", "~"); + name = name.replace("%2F", "/").replace("%2f", "/"); + + if (name.charAt(name.length() - 1) == '/') { + name = name.substring(0, name.length() - 1); + } + + // strip duplicate-slashes from requests for repositoryName (ticket-117, issue-454) + // specify first char as slash so we strip leading slashes + char lastChar = '/'; + StringBuilder sb = new StringBuilder(); + for (char c : name.toCharArray()) { + if (c == '/' && lastChar == c) { + continue; + } + sb.append(c); + lastChar = c; + } + + return sb.toString(); + } + + /** + * Returns the cache key for the repository name. + * + * @param repositoryName + * @return the cache key for the repository + */ + private String getRepositoryKey(String repositoryName) { + String name = fixRepositoryName(repositoryName); + return StringUtils.stripDotGit(name).toLowerCase(); } /** @@ -932,7 +978,8 @@ if (!caseSensitiveCheck && settings.getBoolean(Keys.git.cacheRepositoryList, true)) { // if we are caching use the cache to determine availability // otherwise we end up adding a phantom repository to the cache - return repositoryListCache.containsKey(repositoryName.toLowerCase()); + String key = getRepositoryKey(repositoryName); + return repositoryListCache.containsKey(key); } Repository r = getRepository(repositoryName, false); if (r == null) { @@ -970,7 +1017,7 @@ } String userProject = ModelUtils.getPersonalPath(username); if (settings.getBoolean(Keys.git.cacheRepositoryList, true)) { - String originKey = origin.toLowerCase(); + String originKey = getRepositoryKey(origin); String userPath = userProject + "/"; // collect all origin nodes in fork network @@ -987,7 +1034,7 @@ } if (originModel.originRepository != null) { - String ooKey = originModel.originRepository.toLowerCase(); + String ooKey = getRepositoryKey(originModel.originRepository); roots.add(ooKey); originModel = repositoryListCache.get(ooKey); } else { @@ -1000,7 +1047,8 @@ if (repository.startsWith(userPath)) { RepositoryModel model = repositoryListCache.get(repository); if (!StringUtils.isEmpty(model.originRepository)) { - if (roots.contains(model.originRepository.toLowerCase())) { + String ooKey = getRepositoryKey(model.originRepository); + if (roots.contains(ooKey)) { // user has a fork in this graph return model.name; } @@ -1038,9 +1086,11 @@ public ForkModel getForkNetwork(String repository) { if (settings.getBoolean(Keys.git.cacheRepositoryList, true)) { // find the root, cached - RepositoryModel model = repositoryListCache.get(repository.toLowerCase()); + String key = getRepositoryKey(repository); + RepositoryModel model = repositoryListCache.get(key); while (model.originRepository != null) { - model = repositoryListCache.get(model.originRepository.toLowerCase()); + String originKey = getRepositoryKey(model.originRepository); + model = repositoryListCache.get(originKey); } ForkModel root = getForkModelFromCache(model.name); return root; @@ -1056,7 +1106,8 @@ } private ForkModel getForkModelFromCache(String repository) { - RepositoryModel model = repositoryListCache.get(repository.toLowerCase()); + String key = getRepositoryKey(repository); + RepositoryModel model = repositoryListCache.get(key); if (model == null) { return null; } @@ -1287,7 +1338,7 @@ @Override public void updateRepositoryModel(String repositoryName, RepositoryModel repository, boolean isCreate) throws GitBlitException { - if (gcExecutor.isCollectingGarbage(repositoryName)) { + if (isCollectingGarbage(repositoryName)) { throw new GitBlitException(MessageFormat.format("sorry, Gitblit is busy collecting garbage in {0}", repositoryName)); } @@ -1371,7 +1422,8 @@ // update this repository's origin's fork list if (!StringUtils.isEmpty(repository.originRepository)) { - RepositoryModel origin = repositoryListCache.get(repository.originRepository.toLowerCase()); + String originKey = getRepositoryKey(repository.originRepository); + RepositoryModel origin = repositoryListCache.get(originKey); if (origin != null && !ArrayUtils.isEmpty(origin.forks)) { origin.forks.remove(repositoryName); origin.forks.add(repository.name); @@ -1845,6 +1897,18 @@ } catch (IllegalArgumentException e) { logger.error("Failed to configure JGit parameters!", e); } + + try { + // issue-486/ticket-151: UTF-9 & UTF-18 + Field field = RawParseUtils.class.getDeclaredField("encodingAliases"); + field.setAccessible(true); + Map<String, Charset> encodingAliases = (Map<String, Charset>) field.get(null); + encodingAliases.put("utf-9", RawParseUtils.UTF8_CHARSET); + encodingAliases.put("utf-18", RawParseUtils.UTF8_CHARSET); + logger.info("Alias UTF-9 & UTF-18 encodings as UTF-8 in JGit"); + } catch (Throwable t) { + logger.error("Failed to inject UTF-9 & UTF-18 encoding aliases into JGit", t); + } } protected void configureCommitCache() { -- Gitblit v1.9.1