Skip to content

Commit 8907cfe

Browse files
committed
Scope laboratory assay protocol lookups to the request container
GetImportMethodsAction and four sibling actions in LaboratoryController looked up assay protocols by row id via ExperimentService.getExpProtocol(int), an unscoped global primary-key lookup. Because the actions are guarded only by container-level permissions, a user with read access to any single folder could pass an arbitrary row id and have the server use — and, in GetImportMethodsAction, echo back the name, container, and container path of — a protocol defined in a folder they cannot read, enabling cross-container enumeration of assay designs and folder paths. Each lookup now verifies the protocol is in scope for the request container via AssayService.getAssayProtocols(getContainer()) before use, returning the same generic not-found message whether the row id is unknown or simply out of scope so the response is not an existence oracle. Legitimate same-container callers are unaffected.
1 parent c4d5b64 commit 8907cfe

1 file changed

Lines changed: 29 additions & 6 deletions

File tree

laboratory/src/org/labkey/laboratory/LaboratoryController.java

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -646,8 +646,11 @@ public String getResponse(ProcessAssayForm form, Map<String, Pair<File, String>>
646646
throw new UploadException("No Assay Id Provided", HttpServletResponse.SC_BAD_REQUEST);
647647
}
648648

649+
// getExpProtocol() is a global row-id lookup with no container scoping, so confirm the protocol is in
650+
// scope for this container before using it; otherwise the same generic message is returned whether the
651+
// row id is unknown or simply belongs to a container the user cannot read.
649652
ExpProtocol protocol = ExperimentService.get().getExpProtocol(form.getLabkeyAssayId());
650-
if (protocol == null)
653+
if (protocol == null || !AssayService.get().getAssayProtocols(getContainer()).contains(protocol))
651654
{
652655
throw new UploadException("Unable to find assay protocol with Id: " + form.getLabkeyAssayId(), HttpServletResponse.SC_BAD_REQUEST);
653656
}
@@ -935,8 +938,11 @@ public ApiResponse execute(SaveTemplateForm form, BindException errors) throws E
935938
{
936939
JSONObject json = new JSONObject(form.getJson());
937940

941+
// getExpProtocol() is a global row-id lookup with no container scoping, so confirm the protocol is in
942+
// scope for this container before saving a template against it. The same generic message is returned
943+
// whether the row id is unknown or belongs to a container the user cannot read.
938944
ExpProtocol protocol = ExperimentService.get().getExpProtocol(form.getProtocolId());
939-
if (protocol == null)
945+
if (protocol == null || !AssayService.get().getAssayProtocols(getContainer()).contains(protocol))
940946
{
941947
errors.reject(ERROR_MSG, "Unknown assay: " + form.getProtocolId());
942948
return null;
@@ -1062,8 +1068,11 @@ public void export(ProcessAssayForm form, HttpServletResponse response, BindExce
10621068
return;
10631069
}
10641070

1071+
// getExpProtocol() is a global row-id lookup with no container scoping, so confirm the protocol is in
1072+
// scope for this container before generating a template against it. The same generic message is returned
1073+
// whether the row id is unknown or belongs to a container the user cannot read.
10651074
ExpProtocol protocol = ExperimentService.get().getExpProtocol(form.getLabkeyAssayId());
1066-
if (protocol == null)
1075+
if (protocol == null || !AssayService.get().getAssayProtocols(getContainer()).contains(protocol))
10671076
{
10681077
throw new AbstractFileUploadAction.UploadException("Unable to find assay protocol with Id: " + form.getLabkeyAssayId(), HttpServletResponse.SC_BAD_REQUEST);
10691078
}
@@ -1513,8 +1522,11 @@ public ApiResponse execute(AssayImportHeadersForm form, BindException errors)
15131522
return new ApiSimpleResponse(results);
15141523
}
15151524

1525+
// getExpProtocol() is a global row-id lookup with no container scoping, so confirm the protocol is in scope
1526+
// for this container before returning its import columns. The same generic message is returned whether the
1527+
// row id is unknown or belongs to a container the user cannot read.
15161528
ExpProtocol protocol = ExperimentService.get().getExpProtocol(form.getProtocol());
1517-
if (protocol == null)
1529+
if (protocol == null || !AssayService.get().getAssayProtocols(getContainer()).contains(protocol))
15181530
{
15191531
errors.reject(ERROR_MSG, "Protocol not found: " + form.getProtocol());
15201532
return new ApiSimpleResponse(results);
@@ -1877,8 +1889,19 @@ public ApiResponse execute(ImportMethodsForm form, BindException errors)
18771889
List<ExpProtocol> protocols = new ArrayList<>();
18781890
if (form.getAssayId() != null)
18791891
{
1880-
protocols.add(ExperimentService.get().getExpProtocol(form.getAssayId()));
1881-
ap = AssayService.get().getProvider(protocols.get(0));
1892+
ExpProtocol protocol = ExperimentService.get().getExpProtocol(form.getAssayId());
1893+
// getExpProtocol() is a global row-id lookup with no container scoping. Verify the requested protocol is
1894+
// actually in scope for this container before using or echoing its metadata, otherwise a user with read
1895+
// access to any one folder could enumerate arbitrary row ids and harvest assay names and container paths
1896+
// from folders they cannot read. Use the same generic message whether or not the row id exists so the
1897+
// response is not an existence oracle for protocols in other containers.
1898+
if (protocol == null || !AssayService.get().getAssayProtocols(getContainer()).contains(protocol))
1899+
{
1900+
errors.reject(ERROR_MSG, "Unknown assay: " + form.getAssayId());
1901+
return null;
1902+
}
1903+
protocols.add(protocol);
1904+
ap = AssayService.get().getProvider(protocol);
18821905
}
18831906
else if (form.getAssayType() != null)
18841907
{

0 commit comments

Comments
 (0)