[MGNLDAM-1041] Upload of Zip file containing SVG fails Created: 15/Jun/22  Updated: 06/Jul/22  Resolved: 04/Jul/22

Status: Closed
Project: Magnolia DAM Module
Component/s: None
Affects Version/s: 3.0.16
Fix Version/s: 3.0.18

Type: Bug Priority: Neutral
Reporter: Jonathan Ayala Assignee: Jesus Alonso
Resolution: Fixed Votes: 2
Labels: None
Σ Remaining Estimate: 0d Remaining Estimate: Not Specified
Σ Time Spent: 1.75h Time Spent: 1.5h
Σ Original Estimate: Not Specified Original Estimate: Not Specified

Attachments: Zip Archive samsung-galaxy-a33-128gb.zip     Zip Archive zip-with-svg-assets2.zip    
Issue Links:
causality
duplicate
is duplicated by MGNLDAM-1044 Uploading a file named .extension.zip... Closed
relation
is related to MGNLDAM-1038 SVG validation runs twice Open
Sub-Tasks:
Key
Summary
Type
Status
Assignee
MGNLDAM-1056 QA Sub-task Completed Lam Nguyen Bao  
Template:
Acceptance criteria:
Empty
Task DoD:
[X]* Doc/release notes changes? Comment present?
[X]* Downstream builds green?
[X]* Solution information and context easily available?
[X]* Tests
[X]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:
Epic Link: XSS vulnerability over SVG uploads
Sprint: AuthX 13
Story Points: 1
Team: AuthorX

 Description   

Steps to reproduce

  1. Open assets app
  2. upload zip file zip-with-svg-assets2.zip
  3. upload zip file samsung-galaxy-a33-128gb.zip

Expected results

Both files are correctly uploaded and the assets are created

Actual results

File zip-with-svg-assets.zip is correctly processed whereas samsung-galaxy-a33-128gb.zip throws an error:

info.magnolia.ui.api.action.ActionExecutionException: java.io.IOException: Resetting to invalid mark

Development notes

The error is caused in info.magnolia.dam.app.action.UploadAssetsAction.handleFileEntry(ZipFile, ZipArchiveEntry) line 201:

bis.reset();}

Specifically in line 446 of class BufferedInputStream:

   /**
     * See the general contract of the <code>reset</code>
     * method of <code>InputStream</code>.
     * <p>
     * If <code>markpos</code> is <code>-1</code>
     * (no mark has been set or the mark has been
     * invalidated), an <code>IOException</code>
     * is thrown. Otherwise, <code>pos</code> is
     * set equal to <code>markpos</code>.
     *
     * @exception  IOException  if this stream has not been marked or,
     *                  if the mark has been invalidated, or the stream
     *                  has been closed by invoking its {@link #close()}
     *                  method, or an I/O error occurs.
     * @see        java.io.BufferedInputStream#mark(int)
     */
    public synchronized void reset() throws IOException {
        getBufIfOpen(); // Cause exception if closed
        if (markpos < 0)
            throw new IOException("Resetting to invalid mark");
        pos = markpos;
    }
 

Tested with Java 8 and 14
Tested with different combinations of the files in samsung-galaxy-a33-128gb.zip:

  • Reducing the number of files up to 1
  • Simplifying the file names so they are not long and neither contain characters other than letters.
  • Mixing with the files of the working zip.
    All of the above failed.


 Comments   
Comment by Christoph Damm [ 27/Jun/22 ]

Looks to me like the mentioned code was changed for within magnolia-dam-app-jcr-3.0.13 (making use of bufferedInputStream in combination with SVGValidation and ImageInfo for mime/size file detection), hence broken since magnolia 6.2.16.
In my cases the svg check will set markpos to -1 as well as the checkjpeg method of ImageInfo.

Thus subsequent resets will fail of course. 

 

Why it's sometimes working and sometimes not might depend on readLimit and read bytes by TIKA, but I didn't debug this yet in any detail.

Also mentioned in: https://jira.magnolia-cms.com/browse/MGNLDAM-1044

Generated at Mon Feb 12 05:05:47 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.