OfficeDev / office-js

A repo and NPM package for Office.js, corresponding to a copy of what gets published to the official "evergreen" Office.js CDN, at https://appsforoffice.microsoft.com/lib/1/hosted/office.js.
https://learn.microsoft.com/javascript/api/overview
Other
641 stars 92 forks source link

Excel crashes when trying to read a custom properties with a value that is longer than 255 characters #4378

Closed mburbea closed 2 weeks ago

mburbea commented 1 month ago

The spec for custom properties do indicate that they must be between 1 and 255. However, since an xlsx is an xml document, and vba/3rd com addins may not care it is possible to create a property with such a long payload. When trying to read such a property excel will freeze and then crash.

Your Environment

Current behavior

Excel will freeze up for about 10 seconds and then crash.

Steps to reproduce

you can manually insert a string like the following into the custom properties section of a document.

<property fmtid="{D6F9AF15-B45D-4527-A4EF-3FB3FEAEB2F8}" pid="2" 
  name="BadTime">
<vt:lpwstr>01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789</vt:lpwstr>
</property>

It is likely that vba can do it too.

Context

My addin allows users to create a report and will take screenshots of various workbook name ranges that the user selects. I save into the workbook which ones are currently selected in custom properties prefixed with my addin initials. Thus I load all custom doc properties and then do a regex search for any property. Unfortunately, I've discovered some poorly poorly behaved com addin is storing large base64 strings in the custom properties section of documents. While I can give the users some vba to help find and remove these out of spec properties, it is a poor experience for my users when excel crashes.

dingjin-ms commented 1 month ago

Hi @mburbea , thank you for reporting the issue.

But I'm not able to repro your problem. Here is my repro step:

  1. Save a blank workbook.
  2. Change the custom.xml file with the property string you gave.
  3. Reopen the workbook and load the key value with the script you gave.

I could successfully get the key-value (though just the substring of the actual value).

Could you please provide the file that would crash after loading the custom properties, or could you please share a recording of your complete repro process? Thank you very much.

penglongzhaochina commented 3 weeks ago

Your length of value exceed the 255 characters. larger values are automatically trimmed to 255 characters. This is the official doc to tell this: https://learn.microsoft.com/en-us/javascript/api/excel/excel.customproperty?view=excel-js-preview.

image

The fix is releasing to production, after that your length will be trimmed to 255 characters.

Thanks!

microsoft-github-policy-service[bot] commented 2 weeks ago

This issue has been automatically marked as stale because it is marked as needing author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. Thank you for your interest in Office Add-ins!

microsoft-github-policy-service[bot] commented 2 weeks ago

This issue has been closed due to inactivity. Please comment if you still need assistance and we'll re-open the issue.