abap2xlsx
ATC CVA security finding
Header
ZCL_EXCEL_READER_2007 Method READ_FROM_APPLSERVER Line Number 14 Check Title Security Checks for ABAP (CVA) Check Message Potential directory traversal Priority Priority 1
Body
Operand LV_FILENAME in statement OPEN is a directory traversal risk. Data flow: Class: ZCL_EXCEL_READER_2007 Section: PUBLIC SECTION Method: ZIF_EXCEL_READER~LOAD_FILE Parameter: I_FILENAME I_FILENAME -> I_FILENAME (Method: ZIF_EXCEL_READER~LOAD_FILE Line: 9) I_FILENAME -> LV_FILENAME (Method: READ_FROM_APPLSERVER Line: 11) Cannot be suppressed using a pragma or pseudo-comment
Recommendation is to use logical filenames that have to be validated into physical filenames just before OPEN DATASET. Details refer to 2 SAP notes: 1497003 1957910 or function modules FILE_GET_NAME_AND_VALIDATE FILE_VALIDATE_NAME
I have no idea how to remove this issue without changing the interface of the method. Maybe someone here has some idea.
Thanks. So, if I understand correctly the CVA security finding, it means that after we have fixed it, all people who use it will need to create a Logical File and rewrite their code. No idea what is the best way to inform the people that the new version possibly requires a change in their code. The only way I know is through the Release Notes (whose support has been recently added in abapGit) and that's it.
Moreover, if the people need to rewrite their code, I think that it would be better to get rid of this utility method as we already have load (like abap2xlsx has write_file to return the Xstring but doesn't provide save_file to write the file on application or presentation server). Better have the code of load_file in the demo programs only.
At least according to the CVA message I've found no other option but usage of logical files. If LOAD_FILE is really just an "unused" utility method and LOAD imports an XSTRING which means all the security stuff will be in the calling class/application it seems like a reasonable solution to me to remove LOAD_FILE.