From 8907cfecdf5fdb8c4ea7b68d9fa9dc0a3c58db1f Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Thu, 18 Jun 2026 15:53:50 -0600 Subject: [PATCH 1/2] Scope laboratory assay protocol lookups to the request container MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../laboratory/LaboratoryController.java | 35 +++++++++++++++---- 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/laboratory/src/org/labkey/laboratory/LaboratoryController.java b/laboratory/src/org/labkey/laboratory/LaboratoryController.java index 643a9dc6..0f534fc8 100644 --- a/laboratory/src/org/labkey/laboratory/LaboratoryController.java +++ b/laboratory/src/org/labkey/laboratory/LaboratoryController.java @@ -646,8 +646,11 @@ public String getResponse(ProcessAssayForm form, Map> throw new UploadException("No Assay Id Provided", HttpServletResponse.SC_BAD_REQUEST); } + // getExpProtocol() is a global row-id lookup with no container scoping, so confirm the protocol is in + // scope for this container before using it; otherwise the same generic message is returned whether the + // row id is unknown or simply belongs to a container the user cannot read. ExpProtocol protocol = ExperimentService.get().getExpProtocol(form.getLabkeyAssayId()); - if (protocol == null) + if (protocol == null || !AssayService.get().getAssayProtocols(getContainer()).contains(protocol)) { throw new UploadException("Unable to find assay protocol with Id: " + form.getLabkeyAssayId(), HttpServletResponse.SC_BAD_REQUEST); } @@ -935,8 +938,11 @@ public ApiResponse execute(SaveTemplateForm form, BindException errors) throws E { JSONObject json = new JSONObject(form.getJson()); + // getExpProtocol() is a global row-id lookup with no container scoping, so confirm the protocol is in + // scope for this container before saving a template against it. The same generic message is returned + // whether the row id is unknown or belongs to a container the user cannot read. ExpProtocol protocol = ExperimentService.get().getExpProtocol(form.getProtocolId()); - if (protocol == null) + if (protocol == null || !AssayService.get().getAssayProtocols(getContainer()).contains(protocol)) { errors.reject(ERROR_MSG, "Unknown assay: " + form.getProtocolId()); return null; @@ -1062,8 +1068,11 @@ public void export(ProcessAssayForm form, HttpServletResponse response, BindExce return; } + // getExpProtocol() is a global row-id lookup with no container scoping, so confirm the protocol is in + // scope for this container before generating a template against it. The same generic message is returned + // whether the row id is unknown or belongs to a container the user cannot read. ExpProtocol protocol = ExperimentService.get().getExpProtocol(form.getLabkeyAssayId()); - if (protocol == null) + if (protocol == null || !AssayService.get().getAssayProtocols(getContainer()).contains(protocol)) { throw new AbstractFileUploadAction.UploadException("Unable to find assay protocol with Id: " + form.getLabkeyAssayId(), HttpServletResponse.SC_BAD_REQUEST); } @@ -1513,8 +1522,11 @@ public ApiResponse execute(AssayImportHeadersForm form, BindException errors) return new ApiSimpleResponse(results); } + // getExpProtocol() is a global row-id lookup with no container scoping, so confirm the protocol is in scope + // for this container before returning its import columns. The same generic message is returned whether the + // row id is unknown or belongs to a container the user cannot read. ExpProtocol protocol = ExperimentService.get().getExpProtocol(form.getProtocol()); - if (protocol == null) + if (protocol == null || !AssayService.get().getAssayProtocols(getContainer()).contains(protocol)) { errors.reject(ERROR_MSG, "Protocol not found: " + form.getProtocol()); return new ApiSimpleResponse(results); @@ -1877,8 +1889,19 @@ public ApiResponse execute(ImportMethodsForm form, BindException errors) List protocols = new ArrayList<>(); if (form.getAssayId() != null) { - protocols.add(ExperimentService.get().getExpProtocol(form.getAssayId())); - ap = AssayService.get().getProvider(protocols.get(0)); + ExpProtocol protocol = ExperimentService.get().getExpProtocol(form.getAssayId()); + // getExpProtocol() is a global row-id lookup with no container scoping. Verify the requested protocol is + // actually in scope for this container before using or echoing its metadata, otherwise a user with read + // access to any one folder could enumerate arbitrary row ids and harvest assay names and container paths + // from folders they cannot read. Use the same generic message whether or not the row id exists so the + // response is not an existence oracle for protocols in other containers. + if (protocol == null || !AssayService.get().getAssayProtocols(getContainer()).contains(protocol)) + { + errors.reject(ERROR_MSG, "Unknown assay: " + form.getAssayId()); + return null; + } + protocols.add(protocol); + ap = AssayService.get().getProvider(protocol); } else if (form.getAssayType() != null) { From 7931b293532a82a16862475c0ac6e34b0e4c5544 Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Fri, 19 Jun 2026 06:49:14 -0600 Subject: [PATCH 2/2] Condense container-scoping comments in LaboratoryController --- .../laboratory/LaboratoryController.java | 23 +++++-------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/laboratory/src/org/labkey/laboratory/LaboratoryController.java b/laboratory/src/org/labkey/laboratory/LaboratoryController.java index 0f534fc8..ccaa0dab 100644 --- a/laboratory/src/org/labkey/laboratory/LaboratoryController.java +++ b/laboratory/src/org/labkey/laboratory/LaboratoryController.java @@ -646,9 +646,7 @@ public String getResponse(ProcessAssayForm form, Map> throw new UploadException("No Assay Id Provided", HttpServletResponse.SC_BAD_REQUEST); } - // getExpProtocol() is a global row-id lookup with no container scoping, so confirm the protocol is in - // scope for this container before using it; otherwise the same generic message is returned whether the - // row id is unknown or simply belongs to a container the user cannot read. + // getExpProtocol() is unscoped, so verify the protocol is in scope for this container before using it. ExpProtocol protocol = ExperimentService.get().getExpProtocol(form.getLabkeyAssayId()); if (protocol == null || !AssayService.get().getAssayProtocols(getContainer()).contains(protocol)) { @@ -938,9 +936,7 @@ public ApiResponse execute(SaveTemplateForm form, BindException errors) throws E { JSONObject json = new JSONObject(form.getJson()); - // getExpProtocol() is a global row-id lookup with no container scoping, so confirm the protocol is in - // scope for this container before saving a template against it. The same generic message is returned - // whether the row id is unknown or belongs to a container the user cannot read. + // getExpProtocol() is unscoped, so verify the protocol is in scope for this container before saving a template against it. ExpProtocol protocol = ExperimentService.get().getExpProtocol(form.getProtocolId()); if (protocol == null || !AssayService.get().getAssayProtocols(getContainer()).contains(protocol)) { @@ -1068,9 +1064,7 @@ public void export(ProcessAssayForm form, HttpServletResponse response, BindExce return; } - // getExpProtocol() is a global row-id lookup with no container scoping, so confirm the protocol is in - // scope for this container before generating a template against it. The same generic message is returned - // whether the row id is unknown or belongs to a container the user cannot read. + // getExpProtocol() is unscoped, so verify the protocol is in scope for this container before generating a template against it. ExpProtocol protocol = ExperimentService.get().getExpProtocol(form.getLabkeyAssayId()); if (protocol == null || !AssayService.get().getAssayProtocols(getContainer()).contains(protocol)) { @@ -1522,9 +1516,7 @@ public ApiResponse execute(AssayImportHeadersForm form, BindException errors) return new ApiSimpleResponse(results); } - // getExpProtocol() is a global row-id lookup with no container scoping, so confirm the protocol is in scope - // for this container before returning its import columns. The same generic message is returned whether the - // row id is unknown or belongs to a container the user cannot read. + // getExpProtocol() is unscoped, so verify the protocol is in scope for this container before returning its import columns. ExpProtocol protocol = ExperimentService.get().getExpProtocol(form.getProtocol()); if (protocol == null || !AssayService.get().getAssayProtocols(getContainer()).contains(protocol)) { @@ -1890,11 +1882,8 @@ public ApiResponse execute(ImportMethodsForm form, BindException errors) if (form.getAssayId() != null) { ExpProtocol protocol = ExperimentService.get().getExpProtocol(form.getAssayId()); - // getExpProtocol() is a global row-id lookup with no container scoping. Verify the requested protocol is - // actually in scope for this container before using or echoing its metadata, otherwise a user with read - // access to any one folder could enumerate arbitrary row ids and harvest assay names and container paths - // from folders they cannot read. Use the same generic message whether or not the row id exists so the - // response is not an existence oracle for protocols in other containers. + // getExpProtocol() is unscoped, so verify the protocol is in scope before echoing its metadata; otherwise a user + // could enumerate arbitrary row ids and harvest assay names and container paths from folders they cannot read. if (protocol == null || !AssayService.get().getAssayProtocols(getContainer()).contains(protocol)) { errors.reject(ERROR_MSG, "Unknown assay: " + form.getAssayId());