You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@poi.apache.org by bu...@apache.org on 2017/07/26 18:50:53 UTC

[Bug 61349] New: Add more sanity checks for byte[] allocation

https://bz.apache.org/bugzilla/show_bug.cgi?id=61349

            Bug ID: 61349
           Summary: Add more sanity checks for byte[] allocation
           Product: POI
           Version: 3.17-dev
          Hardware: PC
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: POI Overall
          Assignee: dev@poi.apache.org
          Reporter: tallison@mitre.org
  Target Milestone: ---

Now that I've added sanity checks for byte[] allocation in EMF/WMF, fuzzing is
finding other areas where we might want to do this -- see stacktrace below.

For EMF/WMF, I set some arbitrary max lengths...should we do this more
throughout the codebase to prevent ooms on corrupt files? 


Yet another OOM:

Caused by: java.lang.OutOfMemoryError: Java heap space
        at java.lang.Object.clone(Native Method)
        at
org.apache.poi.ddf.EscherComplexProperty.<init>(EscherComplexProperty.java:46)
        at
org.apache.poi.ddf.EscherPropertyFactory.createProperties(EscherPropertyFactory.java:69)
        at
org.apache.poi.ddf.AbstractEscherOptRecord.fillFields(AbstractEscherOptRecord.java:54)
        at
org.apache.poi.ddf.EscherContainerRecord.fillFields(EscherContainerRecord.java:81)
        at
org.apache.poi.ddf.EscherContainerRecord.fillFields(EscherContainerRecord.java:81)
        at
org.apache.poi.hwpf.model.EscherRecordHolder.fillEscherRecords(EscherRecordHolder.java:56)
        at
org.apache.poi.hwpf.model.EscherRecordHolder.<init>(EscherRecordHolder.java:45)
        at org.apache.poi.hwpf.HWPFDocument.<init>(HWPFDocument.java:280)

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 61349] Add more sanity checks for byte[] allocation

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=61349

--- Comment #2 from Javen O'Neal <on...@apache.org> ---
Should we be making better use of the transient modifier when allocating
performance-related data structures?

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 61349] Add more sanity checks for byte[] allocation

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=61349

--- Comment #1 from Tim Allison <ta...@mitre.org> ---
r1809169 initial commit.

I tried to avoid checks in "serialize()" methods on the theory that the object
has already been collected, it should be good.

I also avoided most checks where there was a copy of an existing array.

We'll likely have to increase some of the thresholds, and I look forward to
running these mods against our regression sets.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 61349] Add more sanity checks for byte[] allocation

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=61349

Dominik Stadler <do...@gmx.at> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #5 from Dominik Stadler <do...@gmx.at> ---
I think this is mostly fixed now, new items can be handled in separate issues
if necessary.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 61349] Add more sanity checks for byte[] allocation

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=61349

Dominik Stadler <do...@gmx.at> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 OS|                            |All

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 61349] Add more sanity checks for byte[] allocation

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=61349

--- Comment #3 from Dominik Stadler <do...@gmx.at> ---
I ran a regression test run with the new limitations. 

Out of 1.1 million documents only 80 differences occurred in 4.0.0 compared to
3.17.

Out of these 19 were OOMs that probably happened before as well.

Thus only 61 documents fail with the new limit. 

I saw that almost all of them are in the 1-2MB range, a few try to allocate a
bit more than 2MB, so if we raise the limit to 2.5MB, we should be safe for
almost all documents of this corpus.

See
http://people.apache.org/~centic/poi_regression/reportsAll/index317to400SNAPSHOT.html
for details.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 61349] Add more sanity checks for byte[] allocation

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=61349

--- Comment #4 from Tim Allison <ta...@mitre.org> ---
Wow.  Thank you, Dominik!  I'm surprised there weren't more problems.

In r1809623, I bumped the following to 10MB:

PPDrawing
PPDrawingGroup
PlexOfCps
ExOleObjStg
ListLevel
SoundData

I bumped the following to 100MB:
EscherBlipRecord

There are still a few records with some pretty big sizes, but, that's the point
of this fix, e.g.: 536,871,012, 2,013,296,702, 1,451,486,230  :)

Thank you, again, Dominik!

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org