From 9f6b02c5d096e648c85dc602a5b28fe1cde0c7da Mon Sep 17 00:00:00 2001
From: jvoisin <jvoisin@users.noreply.github.com>
Date: Thu, 24 Oct 2019 19:39:39 +0000
Subject: [PATCH] Refactor a bit how we're handling avatars

- Remove an unnecessary cast
- Fix two stored XSS, since the name of the avatar is user-controlled
- Tighten the type of some exceptions
---
 .../player/controller/AvatarUploadController.java   | 13 ++++++-------
 .../main/webapp/WEB-INF/jsp/personalSettings.jsp    |  4 ++--
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/airsonic-main/src/main/java/org/airsonic/player/controller/AvatarUploadController.java b/airsonic-main/src/main/java/org/airsonic/player/controller/AvatarUploadController.java
index 94dcdec8..15d8dd91 100644
--- a/airsonic-main/src/main/java/org/airsonic/player/controller/AvatarUploadController.java
+++ b/airsonic-main/src/main/java/org/airsonic/player/controller/AvatarUploadController.java
@@ -43,6 +43,7 @@ import javax.servlet.http.HttpServletRequest;
 import java.awt.image.BufferedImage;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
+import java.io.IOException;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
@@ -78,12 +79,10 @@ public class AvatarUploadController  {
         Map<String, Object> map = new HashMap<String, Object>();
         FileItemFactory factory = new DiskFileItemFactory();
         ServletFileUpload upload = new ServletFileUpload(factory);
-        List<?> items = upload.parseRequest(request);
+        List<FileItem> items = upload.parseRequest(request);
 
         // Look for file items.
-        for (Object o : items) {
-            FileItem item = (FileItem) o;
-
+        for (FileItem item : items) {
             if (!item.isFormField()) {
                 String fileName = item.getName();
                 byte[] data = item.get();
@@ -109,7 +108,7 @@ public class AvatarUploadController  {
         try {
             image = ImageIO.read(new ByteArrayInputStream(data));
             if (image == null) {
-                throw new Exception("Failed to decode incoming image: " + fileName + " (" + data.length + " bytes).");
+                throw new IOException("Failed to decode incoming image: " + fileName + " (" + data.length + " bytes).");
             }
             int width = image.getWidth();
             int height = image.getHeight();
@@ -117,7 +116,7 @@ public class AvatarUploadController  {
 
             // Scale down image if necessary.
             if (width > MAX_AVATAR_SIZE || height > MAX_AVATAR_SIZE) {
-                double scaleFactor = MAX_AVATAR_SIZE / (double) Math.max(width, height);
+                double scaleFactor = MAX_AVATAR_SIZE / (double)Math.max(width, height);
                 height = (int) (height * scaleFactor);
                 width = (int) (width * scaleFactor);
                 image = CoverArtController.scale(image, width, height);
@@ -131,7 +130,7 @@ public class AvatarUploadController  {
             settingsService.setCustomAvatar(avatar, username);
             LOG.info("Created avatar '" + fileName + "' (" + data.length + " bytes) for user " + username);
 
-        } catch (Exception x) {
+        } catch (IOException x) {
             LOG.warn("Failed to upload personal image: " + x, x);
             map.put("error", x);
         }
diff --git a/airsonic-main/src/main/webapp/WEB-INF/jsp/personalSettings.jsp b/airsonic-main/src/main/webapp/WEB-INF/jsp/personalSettings.jsp
index 45d9ef80..0157242f 100644
--- a/airsonic-main/src/main/webapp/WEB-INF/jsp/personalSettings.jsp
+++ b/airsonic-main/src/main/webapp/WEB-INF/jsp/personalSettings.jsp
@@ -220,7 +220,7 @@
             </c:url>
             <span style="white-space:nowrap;">
                 <form:radiobutton id="avatar-${avatar.id}" path="avatarId" value="${avatar.id}"/>
-                <label for="avatar-${avatar.id}"><img src="${avatarUrl}" alt="${avatar.name}" width="${avatar.width}" height="${avatar.height}" style="padding-right:2em;padding-bottom:1em"/></label>
+                <label for="avatar-${avatar.id}"><img src="${avatarUrl}" alt="${fn:escapeXml(avatar.name)}" width="${avatar.width}" height="${avatar.height}" style="padding-right:2em;padding-bottom:1em"/></label>
             </span>
         </c:forEach>
     </p>
@@ -236,7 +236,7 @@
                     <sub:param name="username" value="${command.user.username}"/>
                     <sub:param name="forceCustom" value="true"/>
                 </sub:url>
-                <img src="${avatarUrl}" alt="${command.customAvatar.name}" width="${command.customAvatar.width}" height="${command.customAvatar.height}" style="padding-right:2em"/>
+                <img src="${avatarUrl}" alt="${fn:escapeXml(command.customAvatar.name)}" width="${command.customAvatar.width}" height="${command.customAvatar.height}" style="padding-right:2em"/>
             </c:if>
         </label>
     </p>