Fix for GetImageClippingPathAttribute
Swiss army knife of image processing
Brought to you by:
bfriesen
Menu
▾
▴
1 Attachments
#62 Fix for GetImageClippingPathAttribute
GetImageClippingPathAttribute returns any path present in the Image Resouce Blocks. This patch changes it to check for the presence of the clipping path name (ID=2999). If that's found, it searches for a path with that name. Otherwise, it returns NULL.
How was the allocation size of attr_name selected? Is there a specification which provides the maximum length of a clipping path name (the value part)?
As written, the patch is a recipie for a stack overwrite or reading outside the bounds of a heap allocation if the input is intentionally corrupted.
I am able to make it robust but am curious about the clipping path name length.
The name in the Image Resource Block is a Pascal string, so it's limited to 255. The beginning of the query string is 15 characters. Add a trailing null and 271 is the max. (I made it larger than I needed to in case I needed to add more to the beginning, if I recall correctly. Just being a little paranoid)
Here's the Adobe specification about Image Resource Blocks: https://www.adobe.com/content/dam/acom/en/devnet/photoshop/psir/ps_image_resources.pdf
On Mon, 25 Nov 2019, Phillip Seaver wrote:
As a reminder, a corrupted or non-conforming file could produce any
input. The input could be too short or too long. Null termination
may be missing. More paranoia is required. I have updated my copy
with more paranoia but perhaps not enough yet.
I will take a look.
Bob
Bob Friesenhahn
bfriesen@simple.dallas.tx.us, http://www.simplesystems.org/users/bfriesen/
GraphicsMagick Maintainer, http://www.GraphicsMagick.org/
Public Key, http://www.simplesystems.org/users/bfriesen/public-key.txt
On Mon, 25 Nov 2019, Phillip Seaver wrote:
I did not see a description of a "Pascal string" in Adobe's document.
The only description I see of a string in the document is a string
comprised of 16-bit Unicode characters with a preceding 4-byte size
field. A string like this could be larger than 255.
Are you able to provide a sample file that I can use for testing?
Bob
Bob Friesenhahn
bfriesen@simple.dallas.tx.us, http://www.simplesystems.org/users/bfriesen/
GraphicsMagick Maintainer, http://www.GraphicsMagick.org/
Public Key, http://www.simplesystems.org/users/bfriesen/public-key.txt
On Tue, 26 Nov 2019, Bob Friesenhahn wrote:
If you do provide a sample file, make sure to wrap it in a zip file or
some other archive format which will prevent SourceForge from
re-writing it.
Bob
Bob Friesenhahn
bfriesen@simple.dallas.tx.us, http://www.simplesystems.org/users/bfriesen/
GraphicsMagick Maintainer, http://www.GraphicsMagick.org/
Public Key, http://www.simplesystems.org/users/bfriesen/public-key.txt
I'll have to check with the customer. It was generated by Photoshop, but I don't use it, so I can't tell you the steps. I'll see if I can find out.
Interestingly, I found it used in a few Adobe specs without being defined. It's a one-byte length followed by the string, instead of being null-terminated. https://en.wikipedia.org/wiki/String_(computer_science)#Length-prefixed
A one-byte length does not seem very useful. A variable size length indication (e.g. based on the top bit being set) seems more useful. It seems that we need to learn more about this.
Regardless, I am attaching an alternate patch. Please take a look at it and see if it works for you. I don't plan on committing any fix until I have a test case and have had the time to fully analyze the issue.
Alternative patch
Something similar to this is committed as Mercurial Changeset 17119:d09ea9c70be7. Hopefully all is OK!