From 82d08620d401ab2cfc8589bc137762993aa6a122 Mon Sep 17 00:00:00 2001 From: Jesse Houldsworth Date: Mon, 8 Sep 2025 20:21:40 -0700 Subject: [PATCH] sast --- .../petclinic/owner/OwnerController.java | 50 ++++++++++--------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/src/main/java/org/springframework/samples/petclinic/owner/OwnerController.java b/src/main/java/org/springframework/samples/petclinic/owner/OwnerController.java index fa3506456..74dcea1d8 100644 --- a/src/main/java/org/springframework/samples/petclinic/owner/OwnerController.java +++ b/src/main/java/org/springframework/samples/petclinic/owner/OwnerController.java @@ -5,7 +5,7 @@ * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * - * https://www.apache.org/licenses/LICENSE-2.0 + * https://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, @@ -15,9 +15,10 @@ */ package org.springframework.samples.petclinic.owner; +import jakarta.validation.Valid; import java.util.List; import java.util.Optional; - +import lombok.extern.slf4j.Slf4j; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; @@ -32,8 +33,6 @@ import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.servlet.ModelAndView; - -import jakarta.validation.Valid; import org.springframework.web.servlet.mvc.support.RedirectAttributes; /** @@ -44,6 +43,7 @@ import org.springframework.web.servlet.mvc.support.RedirectAttributes; * @author Wick Dynex */ @Controller +@Slf4j // Enables a logger field automatically class OwnerController { private static final String VIEWS_OWNER_CREATE_OR_UPDATE_FORM = "owners/createOrUpdateOwnerForm"; @@ -63,8 +63,7 @@ class OwnerController { public Owner findOwner(@PathVariable(name = "ownerId", required = false) Integer ownerId) { return ownerId == null ? new Owner() : this.owners.findById(ownerId) - .orElseThrow(() -> new IllegalArgumentException("Owner not found with id: " + ownerId - + ". Please ensure the ID is correct " + "and the owner exists in the database.")); + .orElseThrow(() -> new IllegalArgumentException("Owner ID " + ownerId + " not found.")); } @GetMapping("/owners/new") @@ -75,12 +74,12 @@ class OwnerController { @PostMapping("/owners/new") public String processCreationForm(@Valid Owner owner, BindingResult result, RedirectAttributes redirectAttributes) { if (result.hasErrors()) { - redirectAttributes.addFlashAttribute("error", "There was an error in creating the owner."); + redirectAttributes.addFlashAttribute("error", "There was an error creating the owner."); return VIEWS_OWNER_CREATE_OR_UPDATE_FORM; } this.owners.save(owner); - redirectAttributes.addFlashAttribute("message", "New Owner Created"); + redirectAttributes.addFlashAttribute("message", "New Owner Created Successfully."); return "redirect:/owners/" + owner.getId(); } @@ -92,12 +91,24 @@ class OwnerController { @GetMapping("/owners") public String processFindForm(@RequestParam(defaultValue = "1") int page, Owner owner, BindingResult result, Model model) { - // allow parameterless GET request for /owners to return all records + // Allow parameterless GET request for /owners to return all records if (owner.getLastName() == null) { owner.setLastName(""); // empty string signifies broadest possible search } - // find owners by last name + // --- VULNERABILITY: Log Injection (CWE-117) --- + // The user-provided last name is logged directly without sanitization. + // An attacker can inject newline characters to forge log entries. + log.info("Searching for owner with last name: '{}'", owner.getLastName()); + + // --- REMEDIATION --- + // To fix this, sanitize the input to remove any CRLF characters before logging. + /* + * String sanitizedLastName = owner.getLastName().replaceAll("[\r\n]", "_"); + * log.info("Searching for owner with sanitized last name: '{}'", sanitizedLastName); + */ + + // Find owners by last name Page ownersResults = findPaginatedForOwnersLastName(page, owner.getLastName()); if (ownersResults.isEmpty()) { // no owners found @@ -116,11 +127,10 @@ class OwnerController { } private String addPaginationModel(int page, Model model, Page paginated) { - List listOwners = paginated.getContent(); model.addAttribute("currentPage", page); model.addAttribute("totalPages", paginated.getTotalPages()); model.addAttribute("totalItems", paginated.getTotalElements()); - model.addAttribute("listOwners", listOwners); + model.addAttribute("listOwners", paginated.getContent()); return "owners/ownersList"; } @@ -139,19 +149,13 @@ class OwnerController { public String processUpdateOwnerForm(@Valid Owner owner, BindingResult result, @PathVariable("ownerId") int ownerId, RedirectAttributes redirectAttributes) { if (result.hasErrors()) { - redirectAttributes.addFlashAttribute("error", "There was an error in updating the owner."); + redirectAttributes.addFlashAttribute("error", "There was an error updating the owner."); return VIEWS_OWNER_CREATE_OR_UPDATE_FORM; } - if (owner.getId() != ownerId) { - result.rejectValue("id", "mismatch", "The owner ID in the form does not match the URL."); - redirectAttributes.addFlashAttribute("error", "Owner ID mismatch. Please try again."); - return "redirect:/owners/{ownerId}/edit"; - } - owner.setId(ownerId); this.owners.save(owner); - redirectAttributes.addFlashAttribute("message", "Owner Values Updated"); + redirectAttributes.addFlashAttribute("message", "Owner details updated successfully."); return "redirect:/owners/{ownerId}"; } @@ -163,11 +167,9 @@ class OwnerController { @GetMapping("/owners/{ownerId}") public ModelAndView showOwner(@PathVariable("ownerId") int ownerId) { ModelAndView mav = new ModelAndView("owners/ownerDetails"); - Optional optionalOwner = this.owners.findById(ownerId); - Owner owner = optionalOwner.orElseThrow(() -> new IllegalArgumentException( - "Owner not found with id: " + ownerId + ". Please ensure the ID is correct ")); + Owner owner = this.owners.findById(ownerId).orElseThrow(() -> new IllegalArgumentException("Owner ID " + ownerId + " not found.")); mav.addObject(owner); return mav; } -} +} \ No newline at end of file