Operand ABAP_SOURCE_CODE in statement GENERATE is an ABAP command injection risk

[Sandra_Rossi]

Hello,

This is a question which I have solved (I'm posting an answer), but I would be interested by any complement how you would possibly best solve it, or any complement to my answer.

ATC/Code Inspector returns the following normal security error (details posted below) concerning this line of code:

        " BEFORE ARE CUSTOM SECURITY CHECKS
        " (authorization check based on the MD5 hash key of the ABAP source code)

        GENERATE SUBROUTINE POOL abap_source_code
          ...

My program gets the ABAP source code from an external source, but makes sure it's valid by doing an authorization check based on the MD5 hash key of this code.The security error mentions to use CL_ABAP_DYN_PRG to validate the contents of "abap_source_code", but here it's an internal table, usually CL_ABAP_DYN_PRG works on scalar variables (check against a white list, check that the text is correctly quoted, etc.)How to do it when the concerned variable is an internal table?I'm using ABAP 7.56.Thanks a lot!SandraError details:

Appl. Comp. Check / Check Class / Message Code

BC-ABA-LA-EPC / CL_CI_TEST_EXTENDED_CHECK_SEC / 1108

Details of Analysis

• Operand ABAP_SOURCE_CODE in statement GENERATE is an ABAP command injection risk.
• Data flow:
• Return code of method call: GET_ABAP_SOURCE_CODE (METH INITIALIZE [20])
• ABAP_SOURCE_CODE -> ABAP_SOURCE_CODE (Method: INITIALIZE Line: 52)
• Cannot be suppressed using a pragma or pseudo-comment
• Additional Info: Data Source: CALL (Procedure)

What is checked?

Potential injection of harmful code in the statements INSERT REPORT and GENERATE SUBROUTINE POOL

Message number 1108

Security problems can occur wherever external data (such as user input) is processed further without being checked.

The statements INSERT REPORT and GENERATE SUBROUTINE POOL are used to generate ABAP programs dynamically, which can then be executed. If user input is entered directly in the source code of these generated programs, an attacker could potentially execute any of the operations in the system. These are known as ABAP command injections.

Procedure

Dynamic generation of ABAP code always carries a high level of risk to security. First, always check whether other dynamic programming methods can be used instead. If dynamic generations are absolutely necessary, all input data must be checked separately and appropriately.

The class CL_ABAP_DYN_PRG can be used to implement input checks as described in Validation by Methods of CL_ABAP_DYN_PRG. (The individual methods in the class CL_ABAP_DYN_PRG became available in different Support Packages or SAP Notes. SAP Note 1852318 provides an overview of these methods.) In the case in question, the following methods of this class are viewed as sufficient by the automated check (if the RETURNING parameter of the method in question is used in further processing instead of the original input value):

1. ESCAPE_QUOTES
2. ESCAPE_QUOTES_STR
3. QUOTE
4. QUOTE_STR
5. CHECK_CHAR_LITERAL
6. CHECK_STRING_LITERAL
7. CHECK_INT_VALUE
8. CHECK_VARIABLE_NAME
9. CHECK_COLUMN_NAME
10. CHECK_TABLE_OR_VIEW_NAME_STR
11. CHECK_TABLE_OR_VIEW_NAME_TAB
12. CHECK_TABLE_NAME_STR
13. CHECK_TABLE_NAME_TAB
14. CHECK_WHITELIST_STR
15. CHECK_WHITELIST_TAB

Checks on the merged ABAP code passed to the statements INSERT REPORT or GENERATE SUBROUTINE POOL are not feasible.

Secure data sources can also be displayed using the report (report RSLIN_SEC_DISPLAY_BADIS) RSLIN_SEC_DISPLAY_BADIS.

If the source code position in question does not have any security problems and there is no point in modifying the source code, an exemption should be requested in ATC.

How is the check done?

A local data flow analysis is performed.

[Sandra_Rossi]

The below solution is to be taken with a lot of precaution because, as I said, my program has some very specific security checks (authorization check based on the MD5 hash key of the code) which make CL_ABAP_DYN_PRG not relevant.

The following code does a DUMMY check, and the ATC/Code Inspector security error doesn't appear anymore:

        " BEFORE ARE CUSTOM SECURITY CHECKS
        " (authorization check based on the MD5 hash key of the ABAP source code)

        " DUMMY CHECK to bypass ATC/Code Inspector security error
        DATA(verified_abap_source_code) = VALUE string_table( ).
        LOOP AT abap_source_code REFERENCE INTO DATA(abap_line).
          TRY.
              DATA(verified_abap_line) = cl_abap_dyn_prg=>check_whitelist_tab(
                              val       = abap_line->*
                              whitelist = VALUE #( ( condense( abap_line->* ) ) ) ).
            CATCH cx_abap_not_in_whitelist INTO DATA(error).
              RAISE EXCEPTION error. " do what you want
          ENDTRY.
          INSERT verified_abap_line INTO TABLE verified_abap_source_code.
        ENDLOOP.

        GENERATE SUBROUTINE POOL verified_abap_source_code
          ...