diff --git a/resources/views/scheduleAllInstruments.html b/resources/views/scheduleAllInstruments.html
index 68b8af10e..98948636a 100644
--- a/resources/views/scheduleAllInstruments.html
+++ b/resources/views/scheduleAllInstruments.html
@@ -42,10 +42,7 @@
selectMirror: false,
eventContent: function(arg) {
const e = arg.event;
- const timeFormatString = LABKEY.container.formats.timeFormat
- .replace(':ss', '')
- .replace('.SSS', '');
- const dateStr = DateFormat.format.date(e.start, timeFormatString) + ' - ' + DateFormat.format.date(e.end, timeFormatString);
+ const dateStr = ScheduleUtils.formatTimeRange(e.start, e.end);
const baseColor = e.extendedProps && e.extendedProps.baseColor ? e.extendedProps.baseColor : (arg.backgroundColor || e.backgroundColor || '#888');
const bg = (selectedProjectId && e.extendedProps && e.extendedProps.projectId !== selectedProjectId) ? 'gray' : baseColor;
const textColor = ScheduleUtils.getContrastTextColor(ScheduleUtils.stringToColor(bg));
diff --git a/src/org/labkey/targetedms/TargetedMSModule.java b/src/org/labkey/targetedms/TargetedMSModule.java
index 925e84906..fd5f6ff36 100644
--- a/src/org/labkey/targetedms/TargetedMSModule.java
+++ b/src/org/labkey/targetedms/TargetedMSModule.java
@@ -718,6 +718,7 @@ protected void startupAfterSpringConfig(ModuleContext moduleContext)
ReplicateLabelMinimizer.TestCase.class,
SampleFile.TestCase.class,
SkylineAuditLogParser.TestCase.class,
+ SkylineAuditLogParser.XxeTestCase.class,
TargetedMSController.TestCase.class,
PrecursorManager.TestCase.class,
CrossLinkedPeptideInfo.TestCase.class,
diff --git a/src/org/labkey/targetedms/parser/skyaudit/SkylineAuditLogParser.java b/src/org/labkey/targetedms/parser/skyaudit/SkylineAuditLogParser.java
index 0c38c0784..0e6e7b980 100644
--- a/src/org/labkey/targetedms/parser/skyaudit/SkylineAuditLogParser.java
+++ b/src/org/labkey/targetedms/parser/skyaudit/SkylineAuditLogParser.java
@@ -24,6 +24,8 @@
import org.labkey.api.module.Module;
import org.labkey.api.module.ModuleLoader;
import org.labkey.api.resource.FileResource;
+import org.labkey.api.util.ExternalReferenceProbe;
+import org.labkey.api.util.FileUtil;
import org.labkey.api.util.GUID;
import org.labkey.api.util.Path;
import org.labkey.api.util.XmlBeansUtil;
@@ -31,7 +33,6 @@
import org.labkey.targetedms.parser.XmlUtil;
import org.xml.sax.SAXException;
-import javax.xml.XMLConstants;
import javax.xml.stream.XMLStreamException;
import javax.xml.stream.XMLStreamReader;
import javax.xml.transform.stream.StreamSource;
@@ -45,6 +46,8 @@
import java.io.IOException;
import java.io.InputStream;
import java.math.BigDecimal;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
import java.time.format.DateTimeParseException;
import java.util.Collections;
import java.util.LinkedList;
@@ -114,10 +117,10 @@ private void validateXml() throws IOException, SAXException, AuditLogParsingExce
try (InputStream schemaStream = new BufferedInputStream(openSchemaInputStream());
InputStream auditLogStream = new BufferedInputStream(new FileInputStream(_file)))
{
- //prepare validator
- SchemaFactory schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
+ // Use a factory hardened against XXE
+ SchemaFactory schemaFactory = XmlBeansUtil.schemaFactory();
Schema schema = schemaFactory.newSchema(new StreamSource(schemaStream));
- Validator validator = schema.newValidator();
+ Validator validator = XmlBeansUtil.hardenValidator(schema.newValidator());
validator.validate(new StreamSource(auditLogStream));
}
}
@@ -376,5 +379,65 @@ public void testInvalidXmlFile() throws IOException
//TODO: Validate against different files.
}
+ /**
+ * XXE (CWE-611) coverage for {@link SkylineAuditLogParser#validateXml()}, where the hardening carries the most
+ * weight: unlike the SAML path nothing guards the uploaded .skyl before it reaches the validator, so if the
+ * hardening doesn't hold an uploader can make the server fetch a URL of their choosing.
+ *
+ * Separate from {@link TestCase}, which needs a database for its {@code @Before} cleanup.
+ */
+ public static class XxeTestCase extends Assert
+ {
+ /**
+ * {@link #validateXml} resolves the schema first, so an unavailable module resource means it throws before
+ * parsing any XML and every probe-based assertion below goes green while proving nothing.
+ */
+ @Before
+ public void schemaMustBeResolvable()
+ {
+ Module module = ModuleLoader.getInstance().getModule(TargetedMSModule.class);
+ assertNotNull("TargetedMS module must be registered, otherwise validateXml() never validates", module);
+ assertNotNull("Bundled " + SCHEMA_FILE + " must be resolvable, otherwise validateXml() never validates",
+ module.getModuleResolver().lookup(Path.parse(SCHEMA_FILE)));
+ }
+
+ /** Conforming apart from the injected reference, so validation gets far enough to matter. */
+ private static final String AUDIT_LOG =
+ "" +
+ "hash" +
+ "" +
+ "";
+
+ /**
+ * {@code XmlBeansUtil.TestCase} does primary XXE validation. Just prove that validateXml() gets the hardened factory.
+ */
+ @Test
+ public void validateXmlRefusesExternalDtdSubset() throws Exception
+ {
+ // Qualified because org.labkey.api.util.Path wins the simple name in this file
+ java.nio.file.Path dir = Files.createTempDirectory("skylineAuditLogXxe");
+ try (ExternalReferenceProbe probe = ExternalReferenceProbe.start())
+ {
+ File logFile = dir.resolve("audit.skyl").toFile();
+ Files.writeString(logFile.toPath(),
+ "" + AUDIT_LOG,
+ StandardCharsets.UTF_8);
+
+ // Expected to fail on this input; the assertion is about the fetch on the way there
+ try (SkylineAuditLogParser ignored = new SkylineAuditLogParser(logFile, LogManager.getLogger(XxeTestCase.class)))
+ {
+ fail("Should have failed");
+ }
+ catch (Exception _) {}
+
+ probe.assertNotContacted("Validating an uploaded Skyline audit log must not resolve an external DTD subset");
+ }
+ finally
+ {
+ FileUtil.deleteDir(dir.toFile());
+ }
+ }
+ }
}
diff --git a/src/org/labkey/targetedms/view/instrumentCalendar.jsp b/src/org/labkey/targetedms/view/instrumentCalendar.jsp
index 2e6c35d1a..6fe5b5e73 100644
--- a/src/org/labkey/targetedms/view/instrumentCalendar.jsp
+++ b/src/org/labkey/targetedms/view/instrumentCalendar.jsp
@@ -23,6 +23,7 @@
{
dependencies.add("internal/jQuery");
dependencies.add("targetedms/yearCalendar");
+ dependencies.add("TargetedMS/js/scheduleUtils.js");
}
%>
@@ -51,8 +52,8 @@
$('#delete-event').css('display', event.annotation ? '' : 'none');
$('#event-modal input[name="event-description"]').val(event.annotation ? event.annotation.description : '');
- $('#event-modal input[name="event-start-date"]').val(startDate.getFullYear() + '-' + (startDate.getMonth() + 1 < 10 ? '0' : '') + (startDate.getMonth() + 1) + '-' + (startDate.getDate() < 10 ? '0' : '') + startDate.getDate());
- $('#event-modal input[name="event-end-date"]').val(endDate.getFullYear() + '-' + (endDate.getMonth() + 1 < 10 ? '0' : '') + (endDate.getMonth() + 1) + '-' + (endDate.getDate() < 10 ? '0' : '') + endDate.getDate());
+ $('#event-modal input[name="event-start-date"]').val(ScheduleUtils.toDateValue(startDate));
+ $('#event-modal input[name="event-end-date"]').val(ScheduleUtils.toDateValue(endDate));
$('#annotation-save-error').text('');
$('#event-modal').modal();
}
diff --git a/test/src/org/labkey/test/tests/targetedms/InstrumentSchedulingTest.java b/test/src/org/labkey/test/tests/targetedms/InstrumentSchedulingTest.java
index 1ce61bf63..ea9905ba9 100644
--- a/test/src/org/labkey/test/tests/targetedms/InstrumentSchedulingTest.java
+++ b/test/src/org/labkey/test/tests/targetedms/InstrumentSchedulingTest.java
@@ -183,6 +183,9 @@ public void testSchedule() throws IOException, CommandException
{
String originalStart = getFormElement(START_DATE_TIME_FIELD.findElement(getDriver()));
String originalEnd = getFormElement(END_DATE_TIME_FIELD.findElement(getDriver()));
+ // Pin the 8AM/5PM defaults so a regression that zeroes or shifts the time is caught even on agents whose timezone would otherwise hide it.
+ assertTrue("Start field should default to 8AM, was: " + originalStart, originalStart.endsWith("T08:00"));
+ assertTrue("End field should default to 5PM, was: " + originalEnd, originalEnd.endsWith("T17:00"));
// Try scheduling over the first reservation and verify it is blocked
setFormElement(START_DATE_TIME_FIELD.findElement(getDriver()), originalStart.replace("-03T", "-02T"));
setFormElement(END_DATE_TIME_FIELD.findElement(getDriver()), originalEnd.replace("-03T", "-02T"));
@@ -197,6 +200,9 @@ public void testSchedule() throws IOException, CommandException
assertProjectEventCounts(2, 0);
+ // The event chip time range is rendered via DateFormat (the patched parseTime path); verify it shows the real 8AM-5PM range, not a zeroed 00:00 - 00:00 as happened in colon-labelled timezones.
+ assertEquals("Event chip should show the 8AM-5PM time range", "08:00 - 17:00", getText(Locator.css(".activeProjectEvent .event-date")));
+
doAndWaitForPageToLoad(() -> selectOptionByText(PROJECT_DROP_DOWN, PROJECT_2));
scheduleInstrument(yearMonth + "-04", false);
diff --git a/webapp/TargetedMS/js/scheduleUtils.js b/webapp/TargetedMS/js/scheduleUtils.js
index 02588526f..7b4199c36 100644
--- a/webapp/TargetedMS/js/scheduleUtils.js
+++ b/webapp/TargetedMS/js/scheduleUtils.js
@@ -8,6 +8,18 @@
(function(window) {
const utils = {};
+ const pad = function(n) { return (n < 10 ? '0' : '') + n; };
+
+ // Date-only 'yyyy-MM-dd' wire form from local fields.
+ utils.toDateValue = function(date) {
+ return date.getFullYear() + '-' + pad(date.getMonth() + 1) + '-' + pad(date.getDate());
+ };
+
+ // Format a Date as datetime-local's fixed 'yyyy-MM-ddTHH:mm' wire form from local fields; avoids DateFormat, which zeroes the time in timezones whose label contains a colon (e.g. Honolulu).
+ utils.toDateTimeLocalValue = function(date) {
+ return utils.toDateValue(date) + 'T' + pad(date.getHours()) + ':' + pad(date.getMinutes());
+ };
+
// Convert a CSS color string (named, rgb, hex) to standard 6-digit HEX color (#RRGGBB)
utils.stringToColor = function(color) {
if (!color) return '#888888';
diff --git a/webapp/TargetedMS/js/scheduler.js b/webapp/TargetedMS/js/scheduler.js
index f289542e2..3c790c76b 100644
--- a/webapp/TargetedMS/js/scheduler.js
+++ b/webapp/TargetedMS/js/scheduler.js
@@ -88,10 +88,7 @@ $(function() {
let bgColor = e.event.extendedProps.project === project ? e.backgroundColor : 'gray';
const cl = e.event.extendedProps.project === project ? 'activeProjectEvent' : 'otherProjectEvent';
let textColor = ScheduleUtils.getContrastTextColor(ScheduleUtils.stringToColor(bgColor));
- let timeFormatString = LABKEY.container.formats.timeFormat;
- // Strip seconds and milliseconds
- timeFormatString = timeFormatString.replace(':ss', '').replace('.SSS', '');
- let dateStr = DateFormat.format.date(e.event.start, timeFormatString) + ' - ' + DateFormat.format.date(e.event.end, timeFormatString);
+ let dateStr = ScheduleUtils.formatTimeRange(e.event.start, e.event.end);
let style = 'background-color: ' + LABKEY.Utils.encodeHtml(bgColor) + '; color: ' + LABKEY.Utils.encodeHtml(textColor) + ';' + 'width: 100%';
content += '
'
+ '
' + LABKEY.Utils.encodeHtml(dateStr) + '
'
@@ -396,8 +393,8 @@ $(function() {
endDate.setDate(endDate.getDate() - 1);
}
- let startDateFormatted = DateFormat.format.date(startDate, LABKEY.container.formats.dateTimeFormat);
- let endDateFormatted = DateFormat.format.date(endDate, LABKEY.container.formats.dateTimeFormat);
+ let startDateFormatted = ScheduleUtils.toDateTimeLocalValue(startDate);
+ let endDateFormatted = ScheduleUtils.toDateTimeLocalValue(endDate);
// remove the old event log rows
removeEventLog();
@@ -410,7 +407,6 @@ $(function() {
$('#delete-event').toggle(!!event.id);
$('#add-event').text('Save');
$('#schedule-save-error').text('');
- $('#schedule-cost-error').text('');
$('#event-modal').modal();
}
@@ -519,7 +515,7 @@ $(function() {
}
let fee = data.rows[0].fee;
let rateType = data.rows[0].rateType;
- let cost = fee * (Math.abs(end - start)) / 1000 / 60 / 60;
+ let cost = fee * (end - start) / 1000 / 60 / 60; // the guard above establishes end > start
LABKEY.Query.selectRows({
schemaName: 'targetedms',
@@ -529,6 +525,10 @@ $(function() {
LABKEY.Filter.create('Id', rateType, LABKEY.Filter.Types.EQUAL)
],
success: function (rt) {
+ if (rt.rows.length === 0) {
+ previewErrorEl.text('No rate type found for instrument.');
+ return;
+ }
let setupFee = rt.rows[0].setupFee;
let instrumentFee = Math.round((cost + Number.EPSILON) * 100) / 100;
setupFee = Math.round((setupFee + Number.EPSILON) * 100) / 100;
@@ -581,6 +581,13 @@ $(function() {
});
function fetchInstrumentCosts(instrumentId, startDate, endDate) {
+ // Normalize to Date: callers pass either Date objects (hover) or seconds-less datetime-local strings (post-save), which DateFormat can't parse as-is.
+ const start = new Date(startDate);
+ const end = new Date(endDate);
+ // Bail on degenerate input so the cost log never renders $NaN or garbage dates (mirrors calculateAndRenderCostPreview).
+ if (isNaN(start.getTime()) || isNaN(end.getTime()) || end <= start) {
+ return;
+ }
LABKEY.Query.selectRows({
schemaName: 'targetedms',
queryName: 'instrumentRate',
@@ -595,7 +602,7 @@ $(function() {
}
let fee = data.rows[0].fee;
let rateType = data.rows[0].rateType;
- let cost = fee * (Math.abs(new Date(endDate) - new Date(startDate))) / 1000 / 60 / 60;
+ let cost = fee * (end - start) / 1000 / 60 / 60; // the guard above establishes end > start
// query the rateType
LABKEY.Query.selectRows({
@@ -606,17 +613,19 @@ $(function() {
LABKEY.Filter.create('Id', rateType, LABKEY.Filter.Types.EQUAL)
],
success: function (data) {
+ if (data.rows.length === 0) {
+ $('#schedule-save-error').text('Error calculating cost. No rate type found for instrument');
+ return;
+ }
let setupFee = data.rows[0].setupFee;
- let startDateFormatted = DateFormat.format.date(startDate, LABKEY.container.formats.dateTimeFormat);
- let endDateFormatted = DateFormat.format.date(endDate, LABKEY.container.formats.dateTimeFormat);
+ let startDateFormatted = DateFormat.format.date(start, LABKEY.container.formats.dateTimeFormat);
+ let endDateFormatted = DateFormat.format.date(end, LABKEY.container.formats.dateTimeFormat);
let tableElt = document.getElementById('event-cost-table');
let rowElt = document.createElement('tr');
rowElt.className = 'labkey-row';
let totalCost = Math.round(((setupFee + cost) + Number.EPSILON) * 100) / 100;
- cost = Math.round((cost + Number.EPSILON) * 100) / 100;
- setupFee = Math.round((setupFee + Number.EPSILON) * 100) / 100;
rowElt.innerHTML = '
' + startDateFormatted + ' | ' + endDateFormatted + ' | ' + '$' + totalCost.toLocaleString('en-US', {
minimumFractionDigits: 2,
maximumFractionDigits: 2
@@ -628,22 +637,9 @@ $(function() {
minimumFractionDigits: 2,
maximumFractionDigits: 2
});
-
- $('#setup-cost').val('$' + setupFee.toLocaleString('en-US', {
- minimumFractionDigits: 2,
- maximumFractionDigits: 2
- }));
- $('#instrument-fee').val('$' + cost.toLocaleString('en-US', {
- minimumFractionDigits: 2,
- maximumFractionDigits: 2
- }));
- $('#total-cost').val('$' + (setupFee + cost).toLocaleString('en-US', {
- minimumFractionDigits: 2,
- maximumFractionDigits: 2
- }));
},
failure: function (errorInfo) {
- $('#schedule-cost-error').text('Error calculating cost ' + (errorInfo.exception ? errorInfo.exception : ''));
+ $('#schedule-save-error').text('Error calculating cost ' + (errorInfo.exception ? errorInfo.exception : ''));
}
});
}
|