Backport/8794/stable6.6

lib/Service/Appointments/BookingService.php

@@ -87,9 +87,14 @@ public function __construct(AvailabilityGenerator $availabilityGenerator,
 	 * @throws NoSlotFoundException|ClientException|DbException
 	 */
 	public function confirmBooking(Booking $booking, AppointmentConfig $config): Booking {
-		$bookingSlot = current($this->getAvailableSlots($config, $booking->getStart(), $booking->getEnd()));
+		$availableSlots = $this->getAvailableSlots($config, $booking->getStart(), $booking->getEnd());
+		$selectedSlot = $this->findMatchingSlot(
+			$availableSlots,
+			$booking->getStart(),
+			$booking->getEnd(),
+		);
 
-		if (!$bookingSlot) {
+		if ($selectedSlot === null) {
 			throw new NoSlotFoundException('Slot for booking is not available any more');
 		}
 
@@ -131,9 +136,14 @@ public function confirmBooking(Booking $booking, AppointmentConfig $config): Boo
 	 * @throws ServiceException|DbException|NoSlotFoundException|InvalidArgumentException
 	 */
 	public function book(AppointmentConfig $config, int $start, int $end, string $timeZone, string $displayName, string $email, ?string $description = null): Booking {
-		$bookingSlot = current($this->getAvailableSlots($config, $start, $end));
+		$availableSlots = $this->getAvailableSlots($config, $start, $end);
+		$selectedSlot = $this->findMatchingSlot(
+			$availableSlots,
+			$start,
+			$end,
+		);
 
-		if (!$bookingSlot) {
+		if ($selectedSlot === null) {
 			throw new NoSlotFoundException('Could not find slot for booking');
 		}
 
@@ -150,8 +160,8 @@ public function book(AppointmentConfig $config, int $start, int $end, string $ti
 		$booking->setDisplayName($displayName);
 		$booking->setDescription($description);
 		$booking->setEmail($email);
-		$booking->setStart($start);
-		$booking->setEnd($end);
+		$booking->setStart($selectedSlot->getStart());
+		$booking->setEnd($selectedSlot->getEnd());
 		$booking->setTimezone($tz->getName());
 		try {
 			$this->bookingMapper->insert($booking);
@@ -207,6 +217,21 @@ public function getAvailableSlots(AppointmentConfig $config, int $startTime, int
 		return $available;
 	}
 
+	/**
+	 * Find the slot within $slots whose bounds exactly match $start and $end.
+	 *
+	 * @param Interval[] $slots
+	 */
+	private function findMatchingSlot(array $slots, int $start, int $end): ?Interval {
+		foreach ($slots as $slot) {
+			if ($slot->getStart() === $start && $slot->getEnd() === $end) {
+				return $slot;
+			}
+		}
+
+		return null;
+	}
+
 	/**
 	 * @throws ClientException
 	 */

tests/php/unit/Service/Appointments/BookingServiceTest.php

@@ -15,6 +15,7 @@
 use OCA\Calendar\Db\Booking;
 use OCA\Calendar\Db\BookingMapper;
 use OCA\Calendar\Exception\ClientException;
+use OCA\Calendar\Exception\NoSlotFoundException;
 use OCA\Calendar\Service\Appointments\AvailabilityGenerator;
 use OCA\Calendar\Service\Appointments\BookingCalendarWriter;
 use OCA\Calendar\Service\Appointments\BookingService;
@@ -161,7 +162,7 @@ public function testBookInvalidTimezone(): void {
 			->method('generate');
 
 		$this->expectExceptionObject(new InvalidArgumentException('Could not make sense of the timezone'));
-		$this->service->book(new AppointmentConfig(), 4054546654, 44545454, 'Nighttime/DAYTIME!', 'Test', 'test@test.com', 'Test');
+		$this->service->book(new AppointmentConfig(), 1891378800, 1891382400, 'Nighttime/DAYTIME!', 'Test', 'test@test.com', 'Test');
 	}
 
 	public function testBook(): void {
@@ -197,6 +198,33 @@ public function testBook(): void {
 		$this->service->book(new AppointmentConfig(), $start->getTimestamp(), $end->getTimestamp(), 'Europe/Berlin', 'Test', 'test@test.com');
 	}
 
+	public function testBookSlotMismatch(): void {
+		// Attacker-supplied range that only overlaps a real slot instead of matching one exactly.
+		$start = 1891378800;
+		$end = 1891389600;
+		$intervals = [
+			new Interval(1891382400, 1891386000),
+		];
+
+		$this->availabilityGenerator->expects(self::once())
+			->method('generate')
+			->willReturn($intervals);
+		$this->extrapolator->expects(self::once())
+			->method('extrapolate')
+			->willReturnArgument(1);
+		$this->dailyLimitFilter->expects(self::once())
+			->method('filter')
+			->willReturnArgument(1);
+		$this->eventConflictFilter->expects(self::once())
+			->method('filter')
+			->willReturnArgument(1);
+		$this->bookingMapper->expects(self::never())
+			->method('insert');
+
+		$this->expectException(NoSlotFoundException::class);
+		$this->service->book(new AppointmentConfig(), $start, $end, 'Europe/Berlin', 'Test', 'test@test.com');
+	}
+
 	public function testConfirmBooking(): void {
 		$booking = new Booking();
 		$booking->setStart(1891378800);
@@ -264,6 +292,38 @@ public function testConfirmBookingNoSlot(): void {
 		$this->service->confirmBooking($booking, $config);
 	}
 
+	public function testConfirmBookingSlotMismatch(): void {
+		// The persisted booking's start/end no longer match any currently available slot,
+		// even though the availability check itself returns a non-empty list.
+		$booking = new Booking();
+		$booking->setStart(1891378800);
+		$booking->setEnd(1891382400);
+		$config = new AppointmentConfig();
+		$interval = [
+			new Interval(1891382400, 1891386000),
+		];
+
+		$this->availabilityGenerator->expects(self::once())
+			->method('generate')
+			->willReturn($interval);
+		$this->extrapolator->expects(self::once())
+			->method('extrapolate')
+			->willReturnArgument(1);
+		$this->dailyLimitFilter->expects(self::once())
+			->method('filter')
+			->willReturnArgument(1);
+		$this->eventConflictFilter->expects(self::once())
+			->method('filter')
+			->willReturnArgument(1);
+		$this->bookingCalendarWriter->expects(self::never())
+			->method('write');
+		$this->bookingMapper->expects(self::never())
+			->method('update');
+
+		$this->expectException(NoSlotFoundException::class);
+		$this->service->confirmBooking($booking, $config);
+	}
+
 	public function testFindByToken() {
 		$token = 'test';