Skip to content
Permalink
Browse files

Do not use deserialized AttachmentSupport from client side to avoid s…

…ecurity vulnerabilities
  • Loading branch information
robinshine committed Nov 18, 2020
1 parent a7914f2 commit f864053176c08f59ef2d97fea192ceca46a4d9be
@@ -272,7 +272,6 @@
import io.onedev.server.web.avatar.AvatarManager;
import io.onedev.server.web.avatar.DefaultAvatarManager;
import io.onedev.server.web.component.diff.DiffRenderer;
import io.onedev.server.web.component.markdown.AttachmentUploadServlet;
import io.onedev.server.web.component.markdown.SourcePositionTrackExtension;
import io.onedev.server.web.component.markdown.emoji.EmojiExtension;
import io.onedev.server.web.component.taskbutton.TaskButton;
@@ -557,8 +556,6 @@ private void configureWeb() {
bind(EditSupportRegistry.class).to(DefaultEditSupportRegistry.class);
bind(WebSocketManager.class).to(DefaultWebSocketManager.class);

bind(AttachmentUploadServlet.class);

contributeFromPackage(EditSupport.class, EditSupport.class);

bind(org.apache.wicket.protocol.http.WebApplication.class).to(WebApplication.class);
@@ -93,6 +93,7 @@
import io.onedev.server.web.page.project.setting.build.ActionAuthorizationsPage;
import io.onedev.server.web.page.project.setting.build.BuildPreservationsPage;
import io.onedev.server.web.page.project.setting.build.JobSecretsPage;
import io.onedev.server.web.page.project.setting.general.GeneralProjectSettingPage;
import io.onedev.server.web.page.project.setting.tagprotection.TagProtectionsPage;
import io.onedev.server.web.page.project.setting.webhook.WebHooksPage;
import io.onedev.server.web.page.project.stats.ProjectContribsPage;
@@ -260,7 +261,7 @@ private void addProjectPages() {
add(new DynamicPathPageMapper("projects/${project}/builds/${build}/artifacts", BuildArtifactsPage.class));
add(new DynamicPathPageMapper("projects/${project}/builds/${build}/invalid", InvalidBuildPage.class));

add(new DynamicPathPageMapper("projects/${project}/settings/general", GeneralSecuritySettingPage.class));
add(new DynamicPathPageMapper("projects/${project}/settings/general", GeneralProjectSettingPage.class));
add(new DynamicPathPageMapper("projects/${project}/settings/authorizations", ProjectAuthorizationsPage.class));
add(new DynamicPathPageMapper("projects/${project}/settings/avatar-edit", AvatarEditPage.class));
add(new DynamicPathPageMapper("projects/${project}/settings/branch-protection", BranchProtectionsPage.class));

This file was deleted.

@@ -2,9 +2,11 @@

import static org.apache.wicket.ajax.attributes.CallbackParameter.explicit;

import java.io.IOException;
import java.io.Serializable;
import java.io.UnsupportedEncodingException;
import java.net.URLDecoder;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.HashMap;
@@ -13,8 +15,9 @@

import javax.annotation.Nullable;
import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.apache.commons.lang3.SerializationUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.text.StringEscapeUtils;
import org.apache.wicket.AttributeModifier;
@@ -41,7 +44,8 @@
import org.apache.wicket.request.http.WebRequest;
import org.apache.wicket.request.mapper.parameter.PageParameters;
import org.apache.wicket.request.resource.PackageResourceReference;
import org.apache.wicket.util.crypt.Base64;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.unbescape.javascript.JavaScriptEscape;

import com.fasterxml.jackson.core.JsonProcessingException;
@@ -72,6 +76,8 @@

protected static final int ATWHO_LIMIT = 10;

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

private final boolean compactMode;

private final boolean initialSplit;
@@ -82,7 +88,9 @@

private TextArea<String> input;

private AbstractPostAjaxBehavior ajaxBehavior;
private AbstractPostAjaxBehavior actionBehavior;

private AbstractPostAjaxBehavior attachmentUploadBehavior;

/**
* @param id
@@ -251,7 +259,7 @@ public void renderHead(IHeaderResponse response) {

container.add(new WebMarkupContainer("canAttachFile").setVisible(getAttachmentSupport()!=null));

container.add(ajaxBehavior = new AbstractPostAjaxBehavior() {
container.add(actionBehavior = new AbstractPostAjaxBehavior() {

@Override
protected void updateAjaxAttributes(AjaxRequestAttributes attributes) {
@@ -452,13 +460,42 @@ protected void onClosed() {
String replaceMessage = params.getParameterValue("param2").toString();
String url = getAttachmentSupport().getAttachmentUrl(name);
insertUrl(target, isWebSafeImage(name), url, name, replaceMessage);

break;
default:
throw new IllegalStateException("Unknown action: " + action);
}
}

});

container.add(attachmentUploadBehavior = new AbstractPostAjaxBehavior() {

@Override
protected void respond(AjaxRequestTarget target) {
Preconditions.checkNotNull(getAttachmentSupport(), "Unexpected attachment upload request");
HttpServletRequest request = (HttpServletRequest) RequestCycle.get().getRequest().getContainerRequest();
HttpServletResponse response = (HttpServletResponse) RequestCycle.get().getResponse().getContainerResponse();
try {
String fileName = URLDecoder.decode(request.getHeader("File-Name"), StandardCharsets.UTF_8.name());
String attachmentName = getAttachmentSupport().saveAttachment(fileName, request.getInputStream());
response.getWriter().print(URLEncoder.encode(attachmentName, StandardCharsets.UTF_8.name()));
response.setStatus(HttpServletResponse.SC_OK);
} catch (Exception e) {
logger.error("Error uploading attachment.", e);
try {
if (e.getMessage() != null)
response.getWriter().print(e.getMessage());
else
response.getWriter().print("Internal server error");
} catch (IOException e2) {
throw new RuntimeException(e2);
}
response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
}
}

});
}

@Override
@@ -476,18 +513,9 @@ public void renderHead(IHeaderResponse response) {
super.renderHead(response);
response.render(JavaScriptHeaderItem.forReference(new MarkdownResourceReference()));

String encodedAttachmentSupport;
if (getAttachmentSupport() != null) {
encodedAttachmentSupport = Base64.encodeBase64String(SerializationUtils.serialize(getAttachmentSupport()));
encodedAttachmentSupport = StringUtils.deleteWhitespace(encodedAttachmentSupport);
encodedAttachmentSupport = StringEscapeUtils.escapeEcmaScript(encodedAttachmentSupport);
encodedAttachmentSupport = "'" + encodedAttachmentSupport + "'";
} else {
encodedAttachmentSupport = "undefined";
}

String callback = ajaxBehavior.getCallbackFunction(explicit("action"), explicit("param1"), explicit("param2"),
String actionCallback = actionBehavior.getCallbackFunction(explicit("action"), explicit("param1"), explicit("param2"),
explicit("param3")).toString();
String attachmentUploadUrl = attachmentUploadBehavior.getCallbackUrl().toString();

String autosaveKey = getAutosaveKey();
if (autosaveKey != null)
@@ -497,10 +525,10 @@ public void renderHead(IHeaderResponse response) {

String script = String.format("onedev.server.markdown.onDomReady('%s', %s, %d, %s, %d, %b, %b, '%s', %s);",
container.getMarkupId(),
callback,
actionCallback,
ATWHO_LIMIT,
encodedAttachmentSupport,
getAttachmentSupport()!=null?getAttachmentSupport().getAttachmentMaxSize():0,
getAttachmentSupport()!=null? "'" + attachmentUploadUrl + "'": "undefined",
getAttachmentSupport()!=null? getAttachmentSupport().getAttachmentMaxSize(): 0,
getUserMentionSupport() != null,
getReferenceSupport() != null,
JavaScriptEscape.escapeJavaScript(ProjectNameValidator.PATTERN.pattern()),
@@ -14,7 +14,7 @@ onedev.server.markdown = {
$input[0].dispatchEvent(evt);
}
},
onDomReady: function(containerId, callback, atWhoLimit, attachmentSupport,
onDomReady: function(containerId, callback, atWhoLimit, attachmentUploadUrl,
attachmentMaxSize, canMentionUser, canReferenceEntity,
projectNamePattern, autosaveKey) {
var $container = $("#" + containerId);
@@ -417,7 +417,7 @@ onedev.server.markdown = {
});
}

if (attachmentSupport) {
if (attachmentUploadUrl) {
var inputEl = $input[0];

inputEl.addEventListener("paste", function(e) {
@@ -482,20 +482,27 @@ onedev.server.markdown = {
}

xhr.onload = function() {
var response = xhr.responseText;
var index = response.indexOf("<?xml");
if (index != -1)
response = response.substring(0, index);
if (xhr.status == 200) {
callback("insertUrl", xhr.responseText, xhr.replaceMessage);
callback("insertUrl", response, xhr.replaceMessage);
} else {
onedev.server.markdown.updateUploadMessage($input,
"!!" + xhr.responseText + "!!", xhr.replaceMessage);
"!!" + response + "!!", xhr.replaceMessage);
}
};
xhr.onerror = function() {
onedev.server.markdown.updateUploadMessage($input,
"!!Unable to connect to server!!", xhr.replaceMessage);
};
xhr.open("POST", "/attachment_upload", true);

xhr.open("POST", attachmentUploadUrl, true);
xhr.setRequestHeader("File-Name", encodeURIComponent(file.name));
xhr.setRequestHeader("Attachment-Support", attachmentSupport);
xhr.setRequestHeader("Wicket-Ajax", "true");
xhr.setRequestHeader("Wicket-Ajax-BaseURL", Wicket.Ajax.baseUrl);

xhr.send(file);
}
}
@@ -26,7 +26,6 @@
import io.onedev.server.util.jetty.FileAssetServlet;
import io.onedev.server.util.jetty.ServletConfigurator;
import io.onedev.server.web.asset.icon.IconScope;
import io.onedev.server.web.component.markdown.AttachmentUploadServlet;
import io.onedev.server.web.img.ImageScope;
import io.onedev.server.web.websocket.WebSocketManager;

@@ -44,17 +43,14 @@

private final WicketServlet wicketServlet;

private final AttachmentUploadServlet attachmentUploadServlet;

private final ServletContainer jerseyServlet;

private final WebSocketManager webSocketManager;

@Inject
public ProductServletConfigurator(ServerConfig serverConfig, ShiroFilter shiroFilter, GitFilter gitFilter,
GitPreReceiveCallback preReceiveServlet, GitPostReceiveCallback postReceiveServlet,
WicketServlet wicketServlet, WebSocketManager webSocketManager,
AttachmentUploadServlet attachmentUploadServlet, ServletContainer jerseyServlet) {
WicketServlet wicketServlet, WebSocketManager webSocketManager, ServletContainer jerseyServlet) {
this.serverConfig = serverConfig;
this.shiroFilter = shiroFilter;
this.gitFilter = gitFilter;
@@ -63,7 +59,6 @@ public ProductServletConfigurator(ServerConfig serverConfig, ShiroFilter shiroFi
this.wicketServlet = wicketServlet;
this.webSocketManager = webSocketManager;
this.jerseyServlet = jerseyServlet;
this.attachmentUploadServlet = attachmentUploadServlet;
}

@Override
@@ -88,8 +83,6 @@ public void configure(ServletContextHandler context) {
*/
context.addServlet(new ServletHolder(wicketServlet), "/");

context.addServlet(new ServletHolder(attachmentUploadServlet), "/attachment_upload");

context.addServlet(new ServletHolder(new ClasspathAssetServlet(ImageScope.class)), "/img/*");
context.addServlet(new ServletHolder(new ClasspathAssetServlet(IconScope.class)), "/icon/*");

0 comments on commit f864053

Please sign in to comment.