[Bug]: workspace settings, upload logo API fails when uploading .svg file about appsmith HOT 4 OPEN

@raushan3737 sure you can give it a try

from appsmith.

Hi @somangshu , @Nikhil-Nandagopal i would like to work on this ticket. Can i pick this issue.

from appsmith.

Hi @Nikhil-Nandagopal , @somangshu ,

In order to fix the bug, We have to ensure secure handling of SVG files in the application, we need a comprehensive approach to validate and sanitize SVG content to prevent any cyber attack like(Malicious code injection,Phishing etc).

Below are the approaches and detailed steps to achieve this:

Approach 1: Regex Testing for Malicious Content
Pros: Simple implementation.
Cons: May miss edge cases, leaving potential vulnerabilities as it will not be possible to check all the scenario to prevent malicious script.
Also, there will be a chance to get the wrongly formatted svg during sanitization using regex which might not be svg to render.

Approach 2: Comprehensive Validation and Sanitization

1. Identify Controllers and Services.

2. Validate Image Formats:

• Ensure the application correctly identifies and processes supported image formats (JPEG, PNG, ICO, SVG).
• Special attention should be given to SVG files due to their potential to contain malicious code.

3. SVG Validation and Sanitization:

• Use a library for sanitizing SVG content to ensure it does not contain malicious HTML/JavaScript.
• Consider using OWASP Java HTML Sanitizer , which provides configurable policies to specify allowable content within SVG files & ensure removal of any malicious content in sanitization process. Also, this library is maintain by OWASP team which is the more reputed organization who define & follow the security policy in cyber domain so, we can go with this as we can get future updates also from them.

I feel second approach is more secure way to fix this bug, Should we proceed with approach 2 to use external library for sanitization?

Validation logic file: URL

Sanitization Method(Thinking to use like this):

Backend Code Snippet:

from appsmith.

Hi @Nikhil-Nandagopal , @somangshu ,

While going through above approaches and testing them with multiple svg i found the below observations:

In Approach 2:

• While sanitization we may miss to cover some of the attributes,elements as svg might have various types of attributes, elements to build so, we may lost the actual svg or part of it while sanitization.
• Also, it will increase the jar size as we are thinking to use external library.

In Approach 1:

• Instead of doing the sanitization we can just validate the svg if it passes the validation criteria we will allow that to upload on the server, else we can throw the AppsmithValidationException as please provide the valid svg file.

• By this, we are not allowing user to upload malicious svg also, this implementation will not have much complexity as we are validating through regex pattern.

Snapshot:

Method to validate the svg:

from appsmith.