Skip to content
This repository has been archived by the owner on Aug 20, 2021. It is now read-only.

Commit

Permalink
fix: CVE 623
Browse files Browse the repository at this point in the history 
This closes gravitee-io/issues#2236
  • Loading branch information
@aelamrani @brasseld
aelamrani authored and brasseld committed May 14, 2019
1 parent ec47234 commit 823e467
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 136 deletions.
14 changes: 13 additions & 1 deletion ...agement-api-rest/src/main/java/io/gravitee/management/rest/resource/AbstractResource.java
View file
Expand Up @@ -29,6 +29,9 @@
import javax.inject.Inject;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.SecurityContext;
import java.util.Base64;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

/**
* @author David BRASSELY (david.brassely at graviteesource.com)
Expand All @@ -40,6 +43,7 @@ public abstract class AbstractResource {

public final static String MANAGEMENT_ADMIN = RoleScope.MANAGEMENT.name() + ':' + SystemRole.ADMIN.name();
public final static String PORTAL_ADMIN = RoleScope.PORTAL.name() + ':' + SystemRole.ADMIN.name();
private final static Pattern PATTERN = Pattern.compile("<script");

@Context
protected SecurityContext securityContext;
Expand Down Expand Up @@ -89,8 +93,16 @@ protected boolean hasPermission(RolePermission permission, String referenceId, R
return isAuthenticated() && (isAdmin() || permissionService.hasPermission(permission, referenceId, acls));
}

void checkImageSize(final String picture) {
void checkImage(final String picture) {
if (picture != null) {
// first check that the image is in a valid format to prevent from XSS attack
final String base64Picture = picture.substring(picture.indexOf(',') + 1);
final String decodedPicture = new String(Base64.getDecoder().decode(base64Picture));
final Matcher matcher = PATTERN.matcher(decodedPicture);
if (matcher.find()) {
throw new IllegalArgumentException("The image is not in a valid format");
}
// then check that the image is not too big
final int imageBase64Length = picture.length();
final int approximateImageSizeInByte = 3 * imageBase64Length / 4;
if (approximateImageSizeInByte > 50_000) {
Expand Down
4 changes: 1 addition & 3 deletions ...e-management-api-rest/src/main/java/io/gravitee/management/rest/resource/ApiResource.java
View file
Expand Up @@ -32,7 +32,6 @@
import io.gravitee.management.service.QualityMetricsService;
import io.gravitee.management.service.exceptions.ApiNotFoundException;
import io.gravitee.management.service.exceptions.ForbiddenAccessException;
import io.gravitee.management.service.jackson.ser.api.ApiSerializer;
import io.gravitee.repository.management.model.NotificationReferenceType;
import io.swagger.annotations.*;
import org.glassfish.jersey.message.internal.HttpHeaderReader;
Expand All @@ -47,7 +46,6 @@
import javax.ws.rs.core.Response.Status;
import java.io.ByteArrayOutputStream;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;

Expand Down Expand Up @@ -229,7 +227,7 @@ public Response update(
return builder.build();
}

checkImageSize(apiToUpdate.getPicture());
checkImage(apiToUpdate.getPicture());

final ApiEntity currentApi = (ApiEntity) responseApi.getEntity();
// Force context-path if user is not the primary_owner or an administrator
Expand Down
2 changes: 1 addition & 1 deletion ...ment-api-rest/src/main/java/io/gravitee/management/rest/resource/CurrentUserResource.java
View file
Expand Up @@ -138,7 +138,7 @@ public Response updateCurrentUser(@Valid @NotNull final UpdateUserEntity user) {
if (!user.getUsername().equals(userService.findById(getAuthenticatedUser()).getUsername())) {
throw new ForbiddenAccessException();
}
checkImageSize(user.getPicture());
checkImage(user.getPicture());
return ok(userService.update(user)).build();
}

Expand Down
195 changes: 64 additions & 131 deletions ...nagement-api-rest/src/test/java/io/gravitee/management/rest/resource/ApiResourceTest.java
View file
Expand Up @@ -15,209 +15,142 @@
*/
package io.gravitee.management.rest.resource;

import io.gravitee.common.component.Lifecycle;
import io.gravitee.definition.model.Proxy;
import io.gravitee.management.model.Visibility;
import io.gravitee.management.model.api.ApiEntity;
import io.gravitee.management.model.api.UpdateApiEntity;
import io.gravitee.management.rest.resource.param.LifecycleActionParam;
import org.junit.Ignore;
import io.gravitee.management.service.exceptions.ApiNotFoundException;
import org.junit.Before;
import org.junit.Test;

import javax.ws.rs.client.Entity;
import javax.ws.rs.core.Response;
import java.util.Optional;
import java.util.Date;

import static io.gravitee.common.http.HttpStatusCode.*;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.verify;
import static java.util.Base64.getEncoder;
import static org.apache.commons.lang3.RandomStringUtils.randomAlphanumeric;
import static org.junit.Assert.*;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.*;

/**
* @author David BRASSELY (brasseld at gmail.com)
*/
@Ignore
public class ApiResourceTest extends AbstractResourceTest {

private static final String API_NAME = "my-api";
private static final String API = "my-api";
private static final String UNKNOWN_API = "unknown";

protected String contextPath() {
return "apis/"+API_NAME;
return "apis/";
}

@Test
public void shouldGetApi() {
final ApiEntity mockApi = new ApiEntity();
mockApi.setName(API_NAME);
private ApiEntity mockApi;
private UpdateApiEntity updateApiEntity;

doReturn(mockApi).when(apiService).findById(API_NAME);
@Before
public void init() {
mockApi = new ApiEntity();
mockApi.setId(API);
mockApi.setName(API);
mockApi.setProxy(new Proxy());
mockApi.setUpdatedAt(new Date());
doReturn(mockApi).when(apiService).findById(API);
doThrow(ApiNotFoundException.class).when(apiService).findById(UNKNOWN_API);

updateApiEntity = new UpdateApiEntity();
updateApiEntity.setDescription("toto");
updateApiEntity.setVisibility(Visibility.PUBLIC);
updateApiEntity.setName(API);
updateApiEntity.setVersion("v1");
updateApiEntity.setProxy(new Proxy());
doReturn(mockApi).when(apiService).update(eq(API), any());
}

final Response response = target().request().get();
@Test
public void shouldGetApi() {
final Response response = target(API).request().get();

assertEquals(OK_200, response.getStatus());

final ApiEntity responseApi = response.readEntity(ApiEntity.class);
assertNotNull(responseApi);
assertEquals(API_NAME, responseApi.getName());
assertEquals(API, responseApi.getName());
}

@Test
public void shouldNotGetApiBecauseNotFound() {
final ApiEntity mockApi = new ApiEntity();
mockApi.setName(API_NAME);

doReturn(Optional.empty()).when(apiService).findById(API_NAME);

final Response response = target(API_NAME).request().get();
final Response response = target(UNKNOWN_API).request().get();

assertEquals(NOT_FOUND_404, response.getStatus());
}

@Test
public void shouldNotGetApiBecausePermissionDenied() {
final ApiEntity mockApi = new ApiEntity();
mockApi.setName(API_NAME);

doReturn(new ApiEntity()).when(apiService).findById(API_NAME);

final Response response = target(API_NAME).request().get();

assertEquals(FORBIDDEN_403, response.getStatus());
}

@Test
public void shouldStartApi() {
final ApiEntity mockApi = new ApiEntity();
mockApi.setName(API_NAME);
mockApi.setState(Lifecycle.State.STOPPED);
doReturn(mockApi).when(apiService).start(eq(API), any());

doReturn(Optional.of(mockApi)).when(apiService).findById(API_NAME);

final Response response = target(API_NAME).queryParam("action", LifecycleActionParam.LifecycleAction.START).request().post(null);
final Response response = target(API).queryParam("action", LifecycleActionParam.LifecycleAction.START).request().post(null);

assertEquals(NO_CONTENT_204, response.getStatus());

verify(apiService).start(API_NAME, "admin");
verify(apiService).start(API, "UnitTests");
}

@Test
public void shouldNotStartApiBecauseNotFound() {
final ApiEntity mockApi = new ApiEntity();
mockApi.setName(API_NAME);

doReturn(Optional.empty()).when(apiService).findById(API_NAME);

final Response response = target(API_NAME).queryParam("action", LifecycleActionParam.LifecycleAction.START).request().post(null);
mockApi.setState(Lifecycle.State.STOPPED);
doReturn(mockApi).when(apiService).start(eq(API), any());
final Response response = target(UNKNOWN_API).queryParam("action", LifecycleActionParam.LifecycleAction.START).request().post(null);

assertEquals(NOT_FOUND_404, response.getStatus());
}

@Test
public void shouldNotStartApiBecausePermissionDenied() {
final ApiEntity mockApi = new ApiEntity();
mockApi.setName(API_NAME);

doReturn(Optional.of(mockApi)).when(apiService).findById(API_NAME);

final Response response = target(API_NAME).queryParam("action", LifecycleActionParam.LifecycleAction.START).request().post(null);

assertEquals(FORBIDDEN_403, response.getStatus());
}

@Test
public void shouldStopApi() {
final ApiEntity mockApi = new ApiEntity();
mockApi.setName(API_NAME);

doReturn(Optional.of(mockApi)).when(apiService).findById(API_NAME);
mockApi.setState(Lifecycle.State.STARTED);
doReturn(mockApi).when(apiService).stop(eq(API), any());

final Response response = target(API_NAME).queryParam("action", LifecycleActionParam.LifecycleAction.STOP).request().post(null);
final Response response = target(API).queryParam("action", LifecycleActionParam.LifecycleAction.STOP).request().post(null);

assertEquals(NO_CONTENT_204, response.getStatus());
}

@Test
public void shouldNotStopApiBecauseNotFound() {
final ApiEntity mockApi = new ApiEntity();
mockApi.setName(API_NAME);

doReturn(Optional.empty()).when(apiService).findById(API_NAME);

final Response response = target(API_NAME).queryParam("action", LifecycleActionParam.LifecycleAction.STOP).request().post(null);
final Response response = target(UNKNOWN_API).queryParam("action", LifecycleActionParam.LifecycleAction.STOP).request().post(null);

assertEquals(NOT_FOUND_404, response.getStatus());
}

@Test
public void shouldNotStopApiBecausePermissionDenied() {
final ApiEntity mockApi = new ApiEntity();
mockApi.setName(API_NAME);

doReturn(Optional.of(mockApi)).when(apiService).findById(API_NAME);

final Response response = target(API_NAME).queryParam("action", LifecycleActionParam.LifecycleAction.STOP).request().post(null);

assertEquals(FORBIDDEN_403, response.getStatus());
}

@Test
public void shouldUpdateApi() {
final UpdateApiEntity mockApi = new UpdateApiEntity();
mockApi.setVersion("v1");
mockApi.setDescription("Description of my API");
mockApi.setProxy(new Proxy());

doReturn(new ApiEntity()).when(apiService).update(API_NAME, mockApi);
final Response response = target(API).request().put(Entity.json(updateApiEntity));

final Response response = target(API_NAME).request().put(Entity.json(mockApi));

assertEquals(NO_CONTENT_204, response.getStatus());
assertEquals(response.readEntity(String.class), OK_200, response.getStatus());
}

@Test
public void shouldNotUpdateApiBecausePermissionDenied() {
final UpdateApiEntity mockApi = new UpdateApiEntity();
mockApi.setVersion("v1");
mockApi.setDescription("Description of my API");
mockApi.setProxy(new Proxy());

final Response response = target(API_NAME).request().put(Entity.json(mockApi));
public void shouldNotUpdateApiBecauseTooLargePicture() {
updateApiEntity.setPicture(randomAlphanumeric(100_000));
final Response response = target(API).request().put(Entity.json(updateApiEntity));

assertEquals(FORBIDDEN_403, response.getStatus());
assertEquals(INTERNAL_SERVER_ERROR_500, response.getStatus());
final String message = response.readEntity(String.class);
assertTrue(message, message.contains("The image is too big"));
}

/*
@Test
public void shouldDeleteApi() {
final ApiEntity mockApi = new ApiEntity();
mockApi.setName(API_NAME);
doReturn(Optional.of(mockApi)).when(apiService).findById(API_NAME);
final Response response = target(API_NAME).request().delete();
assertEquals(NO_CONTENT_204, response.getStatus());
verify(apiService).delete(API_NAME);
}
*/

@Test
public void shouldNotDeleteApiBecauseNotFound() {
doReturn(Optional.empty()).when(apiService).findById(API_NAME);

final Response response = target(API_NAME).request().delete();

assertEquals(NOT_FOUND_404, response.getStatus());
}

@Test
public void shouldNotDeleteApiBecausePermissionDenied() {
final ApiEntity mockApi = new ApiEntity();
mockApi.setName(API_NAME);

doReturn(Optional.of(mockApi)).when(apiService).findById(API_NAME);

final Response response = target(API_NAME).request().delete();
public void shouldNotUpdateApiBecauseNotAValidImage() {
updateApiEntity.setPicture(getEncoder().encodeToString("<script>alert('XSS')</script>".getBytes()));
final Response response = target(API).request().put(Entity.json(updateApiEntity));

assertEquals(FORBIDDEN_403, response.getStatus());
assertEquals(INTERNAL_SERVER_ERROR_500, response.getStatus());
final String message = response.readEntity(String.class);
assertTrue(message, message.contains("The image is not in a valid format"));
}
}

0 comments on commit 823e467

Please sign in to comment.