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
master
jvoisin 5 years ago committed by GitHub
parent b37cbf6617
commit 9f6b02c5d0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 13
      airsonic-main/src/main/java/org/airsonic/player/controller/AvatarUploadController.java
  2. 4
      airsonic-main/src/main/webapp/WEB-INF/jsp/personalSettings.jsp

@ -43,6 +43,7 @@ import javax.servlet.http.HttpServletRequest;
import java.awt.image.BufferedImage; import java.awt.image.BufferedImage;
import java.io.ByteArrayInputStream; import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream; import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.util.Date; import java.util.Date;
import java.util.HashMap; import java.util.HashMap;
import java.util.List; import java.util.List;
@ -78,12 +79,10 @@ public class AvatarUploadController {
Map<String, Object> map = new HashMap<String, Object>(); Map<String, Object> map = new HashMap<String, Object>();
FileItemFactory factory = new DiskFileItemFactory(); FileItemFactory factory = new DiskFileItemFactory();
ServletFileUpload upload = new ServletFileUpload(factory); ServletFileUpload upload = new ServletFileUpload(factory);
List<?> items = upload.parseRequest(request); List<FileItem> items = upload.parseRequest(request);
// Look for file items. // Look for file items.
for (Object o : items) { for (FileItem item : items) {
FileItem item = (FileItem) o;
if (!item.isFormField()) { if (!item.isFormField()) {
String fileName = item.getName(); String fileName = item.getName();
byte[] data = item.get(); byte[] data = item.get();
@ -109,7 +108,7 @@ public class AvatarUploadController {
try { try {
image = ImageIO.read(new ByteArrayInputStream(data)); image = ImageIO.read(new ByteArrayInputStream(data));
if (image == null) { 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 width = image.getWidth();
int height = image.getHeight(); int height = image.getHeight();
@ -117,7 +116,7 @@ public class AvatarUploadController {
// Scale down image if necessary. // Scale down image if necessary.
if (width > MAX_AVATAR_SIZE || height > MAX_AVATAR_SIZE) { 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); height = (int) (height * scaleFactor);
width = (int) (width * scaleFactor); width = (int) (width * scaleFactor);
image = CoverArtController.scale(image, width, height); image = CoverArtController.scale(image, width, height);
@ -131,7 +130,7 @@ public class AvatarUploadController {
settingsService.setCustomAvatar(avatar, username); settingsService.setCustomAvatar(avatar, username);
LOG.info("Created avatar '" + fileName + "' (" + data.length + " bytes) for user " + 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); LOG.warn("Failed to upload personal image: " + x, x);
map.put("error", x); map.put("error", x);
} }

@ -220,7 +220,7 @@
</c:url> </c:url>
<span style="white-space:nowrap;"> <span style="white-space:nowrap;">
<form:radiobutton id="avatar-${avatar.id}" path="avatarId" value="${avatar.id}"/> <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> </span>
</c:forEach> </c:forEach>
</p> </p>
@ -236,7 +236,7 @@
<sub:param name="username" value="${command.user.username}"/> <sub:param name="username" value="${command.user.username}"/>
<sub:param name="forceCustom" value="true"/> <sub:param name="forceCustom" value="true"/>
</sub:url> </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> </c:if>
</label> </label>
</p> </p>

Loading…
Cancel
Save