Skip to content

Commit

Permalink
fix: use escaping for html characters
Browse files Browse the repository at this point in the history
  • Loading branch information
mebo4b committed Mar 22, 2021
1 parent 1a6f772 commit 8c81091
Show file tree
Hide file tree
Showing 9 changed files with 155 additions and 131 deletions.
7 changes: 0 additions & 7 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,6 @@
<version>1.15</version>
</dependency>

<!-- Jsoup -->
<dependency>
<groupId>org.jsoup</groupId>
<artifactId>jsoup</artifactId>
<version>1.10.2</version>
</dependency>

<!-- Liquibase -->
<dependency>
<groupId>org.liquibase</groupId>
Expand Down
69 changes: 0 additions & 69 deletions src/main/java/de/caritas/cob/messageservice/api/helper/Helper.java

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public static Optional<String> convertAliasMessageDTOToString(AliasMessageDTO al
try {
return Optional
.ofNullable(
Helper.urlEncodeString(new ObjectMapper().writeValueAsString(aliasMessageDTO)));
UrlEncodingDecodingUtils.urlEncodeString(new ObjectMapper().writeValueAsString(aliasMessageDTO)));
} catch (JsonProcessingException jsonEx) {
LogService.logInternalServerError("Could not convert AliasMessageDTO to alias String",
jsonEx);
Expand All @@ -44,7 +44,7 @@ public static Optional<ForwardMessageDTO> convertStringToForwardMessageDTO(Strin
try {
return Optional
.ofNullable(
new ObjectMapper().readValue(Helper.urlDecodeString(alias), ForwardMessageDTO.class));
new ObjectMapper().readValue(UrlEncodingDecodingUtils.urlDecodeString(alias), ForwardMessageDTO.class));
} catch (IOException jsonParseEx) {
// This is not an error any more due to restructuring of the alias object. This is not a
// real error, but necessary due to legacy code
Expand All @@ -62,7 +62,7 @@ public static Optional<AliasMessageDTO> convertStringToAliasMessageDTO(String al
try {
return Optional
.ofNullable(
new ObjectMapper().readValue(Helper.urlDecodeString(alias), AliasMessageDTO.class));
new ObjectMapper().readValue(UrlEncodingDecodingUtils.urlDecodeString(alias), AliasMessageDTO.class));

} catch (IOException jsonParseEx) {
LogService.logInternalServerError("Could not convert alias String to AliasMessageDTO",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package de.caritas.cob.messageservice.api.helper;

import java.io.UnsupportedEncodingException;
import java.net.URLDecoder;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;

/**
* Util class to provide url encoding and decoding.
*/
public class UrlEncodingDecodingUtils {

private UrlEncodingDecodingUtils() {}

/**
* URL encoding for a given string.
*
* @return the encoded string
*/
public static String urlEncodeString(String stringToEncode) {
try {
return URLEncoder.encode(stringToEncode, StandardCharsets.UTF_8.name());

} catch (UnsupportedEncodingException ex) {
return null;
}
}

/**
* Url decoding for a given string.
*
* @return the decoded string
*/
public static String urlDecodeString(String stringToDecode) {
try {
return URLDecoder.decode(stringToDecode, StandardCharsets.UTF_8.name());

} catch (UnsupportedEncodingException ex) {
return null;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package de.caritas.cob.messageservice.api.helper;

import static org.apache.commons.lang3.StringUtils.EMPTY;
import static org.apache.commons.lang3.StringUtils.isNotBlank;

import org.springframework.web.util.HtmlUtils;

/**
* Provides escaping of html characters for XSS protection.
*/
public class XssProtection {

private XssProtection() {
}

/**
* Escape HTML code from a text (XSS-Protection)
*
* @return the given text without html
*/
public static String escapeHtml(String text) {
if (isNotBlank(text)) {
return HtmlUtils.htmlEscape(text);
}
return EMPTY;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import de.caritas.cob.messageservice.api.exception.NoMasterKeyException;
import de.caritas.cob.messageservice.api.exception.RocketChatBadRequestException;
import de.caritas.cob.messageservice.api.exception.RocketChatUserNotInitializedException;
import de.caritas.cob.messageservice.api.helper.Helper;
import de.caritas.cob.messageservice.api.helper.XssProtection;
import de.caritas.cob.messageservice.api.helper.JSONHelper;
import de.caritas.cob.messageservice.api.model.AliasMessageDTO;
import de.caritas.cob.messageservice.api.model.MessageStreamDTO;
Expand Down Expand Up @@ -181,7 +181,7 @@ public PostMessageResponseDTO postGroupMessage(
throws CustomCryptoException {

// XSS-Protection
message = Helper.removeHTMLFromText(message);
message = XssProtection.escapeHtml(message);
message = encryptionService.encrypt(message, rcGroupId);

try {
Expand Down Expand Up @@ -299,4 +299,4 @@ public GetGroupInfoDto getGroupInfo(String rcToken, String rcUserId, String rcGr
LogService::logRocketChatBadRequestError);
}
}
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package de.caritas.cob.messageservice.api.helper;

import static org.junit.Assert.assertEquals;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.junit.MockitoJUnitRunner;

@RunWith(MockitoJUnitRunner.class)
public class UrlEncodingDecodingUtilsTest {

private static final String DECODED_STRING = "töst#$";
private static final String ENCODED_STRING = "t%C3%B6st%23%24";

@Test
public void urlEncodeString_Should_ReturnEncodedString() {
assertEquals(ENCODED_STRING, UrlEncodingDecodingUtils.urlEncodeString(DECODED_STRING));
}

@Test
public void urlDecodeString_Should_ReturnEncodedString() {
assertEquals(DECODED_STRING, UrlEncodingDecodingUtils.urlDecodeString(ENCODED_STRING));
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package de.caritas.cob.messageservice.api.helper;

import static de.caritas.cob.messageservice.api.helper.XssProtection.escapeHtml;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;

import org.junit.Test;

public class XssProtectionTest {

@Test
public void escapeHtml_Should_escapeStandardHtml() {
String normalized = escapeHtml("<strong>Lorem Ipsum</strong>");

assertThat(normalized, is("&lt;strong&gt;Lorem Ipsum&lt;/strong&gt;"));
}

@Test
public void escapeHtml_Should_escapeJavascriptCode() {
String normalized = escapeHtml("Lorem Ipsum<script>alert('1');</script>");

assertThat(normalized, is("Lorem Ipsum&lt;script&gt;alert(&#39;1&#39;);&lt;/script&gt;"));
}

@Test
public void escapeHtml_ShouldNot_removeNewlinesFromText() {
String normalized = escapeHtml("Lorem Ipsum\nLorem Ipsum");

assertThat(normalized, is("Lorem Ipsum\nLorem Ipsum"));
}

@Test
public void escapeHtml_Should_escapeHtmlFromText_And_ShouldNot_removeNewlines() {
String normalized = escapeHtml("<b>Lorem Ipsum</b>\nLorem Ipsum<script>alert('1');"
+ "</script>");

assertThat(normalized, is("&lt;b&gt;Lorem Ipsum&lt;/b&gt;\nLorem Ipsum&lt;script&gt;alert(&#39;1&#39;);&lt;/script&gt;"));
}

@Test
public void removeHTMLFromText_Should_returnEmptyString_When_inputIsNull() {
String normalized = escapeHtml(null);

assertThat(normalized, is(""));
}

@Test
public void removeHTMLFromText_Should_returnEmptyString_When_inputIsEmptyString() {
String normalized = escapeHtml("");

assertThat(normalized, is(""));
}

}

0 comments on commit 8c81091

Please sign in to comment.