Skip to content

EMPT-41: Prevent upload of executable and script files. #54

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,13 @@
*/
package org.openmrs.module.attachments;

import java.util.List;
import java.util.Arrays;

public class AttachmentsConstants {

public static enum ContentFamily {
EXECUTABLE,
IMAGE,
PDF,
OTHER
Expand Down Expand Up @@ -69,6 +73,12 @@ public static enum ContentFamily {

public static final String UNKNOWN_MIME_TYPE = "application/octet-stream";

public static final List<String> EXECUTABLE_MIME_TYPES = Arrays.asList("application/vnd.microsoft.portable-executable",
"type/javascript", "application/javascrip", "application/x-sh", "application/java-archive",
"application/x-httpd-php", "application/xhtml+xml", "application/x-vbs", "text/vbscript", "text/x-python",
"application/x-ms-installer", "application/x-elf", "application/x-applescript", "application/x-ruby",
"application/x-perl", "application/wasm");

public static final String ATT_VIEW_ORIGINAL = "complexdata.view.original";

public static final String ATT_VIEW_THUMBNAIL = "complexdata.view.thumbnail";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,10 @@ public static double getCompressionRatio(double fileByteSize, double maxByteSize
*/
public static ContentFamily getContentFamily(String mimeType) {
ContentFamily contentFamily = ContentFamily.OTHER;
if (AttachmentsConstants.EXECUTABLE_MIME_TYPES.contains(mimeType)) {
contentFamily = ContentFamily.EXECUTABLE;
}

if (StringUtils.equals(mimeType, "application/pdf")) {
contentFamily = ContentFamily.PDF;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ public class AttachmentResource1_10 extends DataDelegatingCrudResource<Attachmen
private ComplexObsSaver obsSaver = Context.getRegisteredComponent(AttachmentsConstants.COMPONENT_COMPLEXOBS_SAVER,
ComplexObsSaver.class);

private AttachmentsContext ctx = Context
.getRegisteredComponent(AttachmentsConstants.COMPONENT_ATT_CONTEXT, AttachmentsContext.class);
private AttachmentsContext ctx = Context.getRegisteredComponent(AttachmentsConstants.COMPONENT_ATT_CONTEXT,
AttachmentsContext.class);

@Override
public Attachment newDelegate() {
Expand Down Expand Up @@ -112,7 +112,7 @@ public Object upload(MultipartFile file, RequestContext context) throws Response
file = new Base64MultipartFile(base64Content);
}
// Verify File Size
if (ctx.getMaxUploadFileSize() * 1024 * 1024 < (double)file.getSize()) {
if (ctx.getMaxUploadFileSize() * 1024 * 1024 < (double) file.getSize()) {
throw new IllegalRequestException("The file exceeds the maximum size");
}

Expand Down Expand Up @@ -147,6 +147,9 @@ public Object upload(MultipartFile file, RequestContext context) throws Response
obs = obsSaver.saveImageAttachment(visit, patient, encounter, fileCaption, file, instructions);
break;

case EXECUTABLE:
throw new IllegalRequestException("File type is not allowed as attachment.");

case OTHER:
default:
obs = obsSaver.saveOtherAttachment(visit, patient, encounter, fileCaption, file, instructions);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,27 @@ public void postAttachment_shouldNotUploadFileAboveSizeLimit() throws Exception
SimpleObject response = deserialize(handle(request));
}

@Test(expected = IllegalRequestException.class)
public void postAttachment_shouldThrowOnExecutableFile() throws Exception {
// Setup
String dotExeMimeType = "application/vnd.microsoft.portable-executable";
String fileCaption = "Test file caption";
String fileName = "testFile1.dat";
Patient patient = Context.getPatientService().getPatient(2);
Visit visit = Context.getVisitService().getVisit(1);

MockMultipartHttpServletRequest request = newUploadRequest(getURI());
MockMultipartFile dotExeFile = new MockMultipartFile("file", fileName, dotExeMimeType, randomData);

request.addFile(dotExeFile);
request.addParameter("patient", patient.getUuid());
request.addParameter("visit", visit.getUuid());
request.addParameter("fileCaption", fileCaption);

// Replay
SimpleObject response = deserialize(handle(request));
}

@Test
public void getAttachmentBytes_shouldDownloadFile() throws Exception {

Expand Down