From 5ecbbc640231dee6b7ace3f94e98fe4c7fdf022a Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Wed, 17 Jun 2026 05:43:08 -0600 Subject: [PATCH] Scope updateSoftwareRelease to the caller's container MothershipManager.updateSoftwareRelease did a raw Table.update keyed only on the primary key with no container filter, and set the bean's container to the caller's folder. A user with UpdatePermission in one folder could therefore edit and re-home a SoftwareRelease owned by another folder via an attacker-supplied softwareReleaseId on the bean-bound update form. This adds a container-scoped getSoftwareRelease lookup and verifies the target row belongs to the caller's container before updating, throwing NotFoundException otherwise. Also fix an incidental NPE in BulkUpdateAction where updateExceptionStackTrace was called even when the container-scoped lookup returned null; the call now happens only inside the null check. --- .../labkey/mothership/MothershipController.java | 2 +- .../org/labkey/mothership/MothershipManager.java | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/mothership/src/org/labkey/mothership/MothershipController.java b/mothership/src/org/labkey/mothership/MothershipController.java index f3284469b78..46daa25d50c 100644 --- a/mothership/src/org/labkey/mothership/MothershipController.java +++ b/mothership/src/org/labkey/mothership/MothershipController.java @@ -1586,8 +1586,8 @@ else if (!form.isIgnore()) { exceptionStackTrace.setBugNumber(-1); } + MothershipManager.get().updateExceptionStackTrace(exceptionStackTrace, getUser()); } - MothershipManager.get().updateExceptionStackTrace(exceptionStackTrace, getUser()); } catch (NumberFormatException e) { diff --git a/mothership/src/org/labkey/mothership/MothershipManager.java b/mothership/src/org/labkey/mothership/MothershipManager.java index f5a9c22f104..fc4c7017fe4 100644 --- a/mothership/src/org/labkey/mothership/MothershipManager.java +++ b/mothership/src/org/labkey/mothership/MothershipManager.java @@ -42,6 +42,7 @@ import org.labkey.api.util.MothershipReport; import org.labkey.api.util.ReentrantLockWithName; import org.labkey.api.util.logging.LogHelper; +import org.labkey.api.view.NotFoundException; import java.io.IOException; import java.util.ArrayList; @@ -211,6 +212,13 @@ public SoftwareRelease ensureSoftwareRelease(Container container, String revisio } } + public SoftwareRelease getSoftwareRelease(int softwareReleaseId, Container container) + { + SimpleFilter filter = SimpleFilter.createContainerFilter(container); + filter.addCondition(FieldKey.fromString("SoftwareReleaseId"), softwareReleaseId); + return new TableSelector(getTableInfoSoftwareRelease(), filter, null).getObject(SoftwareRelease.class); + } + public ServerInstallation getServerInstallation(@NotNull String serverGUID, @NotNull String serverHostName, @NotNull Container c) { SimpleFilter filter = SimpleFilter.createContainerFilter(c); @@ -597,6 +605,12 @@ public void setStatusCakeApiKey(String statusCakeApiKey) public void updateSoftwareRelease(Container container, User user, SoftwareRelease bean) { + // Verify the target row actually belongs to this container before updating. The raw Table.update below is + // keyed only on the primary key, so without this check a user with UpdatePermission in one folder could edit + // (and, via setContainer, re-home) a SoftwareRelease owned by another folder. + if (getSoftwareRelease(bean.getSoftwareReleaseId(), container) == null) + throw new NotFoundException("SoftwareRelease not found in this folder: " + bean.getSoftwareReleaseId()); + bean.setContainer(container.getId()); Table.update(user, getTableInfoSoftwareRelease(), bean, bean.getSoftwareReleaseId()); }