El código original permitía eliminar un post mediante una petición GET, lo cual es vulnerable a ataques de Cross-Site Request Forgery (CSRF). Un atacante podría engañar a un usuario autenticado para que haga clic en un enlace malicioso que envíe una solicitud GET para eliminar un post sin su consentimiento.
Código vulnerable:
<a href="/posts/{{post.id}}/delete" class="btn">Delete Post</a>@GetMapping("/posts/{id}/delete")
public String deletePost(@PathVariable long id) {
checkPostOwnership(id);
postService.deletePost(id);
return "redirect:/";
}Código corregido:
<form action="/posts/{{post.id}}/delete" method="post">
<input type="hidden" name="_csrf" value="{{_csrf.token}}" />
<button type="submit" class="btn btn-danger">Delete Post</button>
</form>@PostMapping("/posts/{id}/delete")
public String deletePost(@PathVariable long id) {
checkPostOwnership(id);
postService.deletePost(id);
return "redirect:/";
}El código original permitía a los usuarios solicitar cualquier archivo del sistema de archivos del servidor a través de la ruta /api/images?filename=, lo que podría ser explotado para acceder a archivos sensibles o críticos del sistema.
Código vulnerable:
@GetMapping("/api/images")
public ResponseEntity<Resource> getImage(@RequestParam String filename) throws IOException {
Resource file = imageService.getImageFile(filename);
return ResponseEntity.ok()
.header(HttpHeaders.CONTENT_TYPE, imageService.getContentType(filename))
.body(file);
}Código corregido:
@GetMapping("/api/posts/{id}/image")
public ResponseEntity<Resource> showPostImage(@PathVariable long id, Model model) throws IOException {
Resource imageFile = imageService.getImageFile(postService.getPost(id).getImagePath());
MediaType mediaType = MediaTypeFactory
.getMediaType(imageFile)
.orElse(MediaType.IMAGE_JPEG);
return ResponseEntity
.ok()
.contentType(mediaType)
.body(imageFile);
}Soluciones que realicen comprobaciones sobre el nombre del archivo o la ruta en el servicio también serían válidas (y evitan, por ejemplo, la vulnerabilidad siguiente), pero esta solución es más rápida/sencilla y soluciona esta vulnerabilidad concreta, por lo que se considera una solución válida para el examen.
En el código original, al crear un post, se permitía que el usuario estableciera cualquier valor para el campo imagePath, lo que podría ser explotado para realizar un ataque de Path Traversal y acceder a archivos sensibles del servidor en peticiones posteriores.
Código vulnerable:
@PostMapping("/")
public ResponseEntity<Post> createPost(@RequestBody Post post) {
Post createdPost = postService.createPost(post);
...
public Post createPost(Post newPost) {
newPost.setUser(userService.getLoggedUser());
postRepository.save(newPost);
return newPost;
}Código corregido:
public Post createPost(Post newPost) {
Post post = new Post();
post.setTitle(newPost.getTitle());
post.setContent(newPost.getContent());
post.setUser(userService.getLoggedUser());
postRepository.save(post);
return post;
}Esta solución es más sencilla, pero soluciones basadas en el uso de DTOs o validaciones sobre el campo imagePath también serían válidas. Si se aplicó una solución en el servicio de imágenes para evitar la vulnerabilidad de Path Traversal en la lectura de archivos, esta vulnerabilidad ya no sería explotable, aunque es recomendable aplicar una solución como esta para evitar que se puedan establecer valores no válidos en el campo imagePath que podrían causar errores o problemas en otras partes del código.
El código original no verifica en la API REST si el usuario autenticado era el propietario del post antes de permitirle realizar acciones como editar o eliminar un post, lo que podría ser explotado para modificar o eliminar posts de otros usuarios. Lo mismo aplica para los operaciones sobre las imágenes de la API REST.
A continuación se muestra un ejemplo del caso de la edición de un post, pero el mismo principio se aplica para la eliminación de posts y para las operaciones sobre las imágenes. Tal y cómo se comentó durante el examen y para que no hubiera lugar a dudas, esta vulnerabilidad se considera como una única. La solución, en todos los casos, es la misma: usar el método checkPostOwnership para verificar que el usuario autenticado es el propietario del post antes de permitirle realizar la acción, preferiblemente en el servicio para evitar que se pueda saltar esta verificación desde otros controladores o servicios.
Código vulnerable:
@PutMapping("/{id}")
public Post replacePost(@PathVariable long id, @RequestBody Post updatedPost) {
return postService.replacePost(id, updatedPost);
}Código corregido: La mejor opción es moverlo al servicio, para evitar que se pueda saltar la verificación de propiedad desde otros controladores o servicios.
public Post replacePost(long id, Post updatedPost) {
checkPostOwnership(postId);
Post post = postRepository.findById(id).orElseThrow();
updatedPost.setId(id);
updatedPost.setUser(userService.getLoggedUser());
updatedPost.setImagePath(post.getImagePath());
postRepository.save(updatedPost);
return updatedPost;
}