Uploaded image for project: 'Apache NiFi'
  1. Apache NiFi
  2. NIFI-4125

Add basic security settings to TransformXml

Details

    • Improvement
    • Status: Resolved
    • Minor
    • Resolution: Fixed
    • 1.3.0
    • None
    • Core Framework

    Description

      Since data flows can generally deal with non-trusted data, the processors should handle it in a secure manner.

      In case of XML there are various known vulnerabilities - OWASP. Some can be mitigated via XML parser/XSLT Processor features.

      The TransformXml processor should have a setting enabling these secure settings.

      Attachments

        Issue Links

          Activity

            githubbot ASF GitHub Bot added a comment -

            GitHub user yuri1969 opened a pull request:

            https://github.com/apache/nifi/pull/1946

            NIFI-4125 - Add basic security settings to TransformXml

            Thank you for submitting a contribution to Apache NiFi.

            In order to streamline the review of the contribution we ask you
            to ensure the following steps have been taken:

                1. For all changes:
            • [x] Is there a JIRA ticket associated with this PR? Is it referenced
              in the commit message?
            • [x] Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
            • [x] Has your PR been rebased against the latest commit within the target branch (typically master)?
            • [x] Is your initial contribution a single, squashed commit?
                1. For code changes:
            • [x] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder?
            • [ ] Have you written or updated unit tests to verify your changes?
            • [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
            • [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
            • [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
            • [ ] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?
                1. For documentation related changes:
            • [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
                1. Note:
                  Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.

            You can merge this pull request into a Git repository by running:

            $ git pull https://github.com/yuri1969/nifi XSLT-securing

            Alternatively you can review and apply these changes as the patch at:

            https://github.com/apache/nifi/pull/1946.patch

            To close this pull request, make a commit to your master/trunk branch
            with (at least) the following in the commit message:

            This closes #1946


            commit 9469c21f3f3dc3ba137e258e3be711cb2ea5a14d
            Author: yuri1969 <1969yuri1969@gmail.com>
            Date: 2017-06-25T20:10:31Z

            Initial propose


            githubbot ASF GitHub Bot added a comment - GitHub user yuri1969 opened a pull request: https://github.com/apache/nifi/pull/1946 NIFI-4125 - Add basic security settings to TransformXml Thank you for submitting a contribution to Apache NiFi. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: For all changes: [x] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? [x] Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character. [x] Has your PR been rebased against the latest commit within the target branch (typically master)? [x] Is your initial contribution a single, squashed commit? For code changes: [x] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder? [ ] Have you written or updated unit tests to verify your changes? [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0] ( http://www.apache.org/legal/resolved.html#category-a)? [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly? [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly? [ ] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties? For documentation related changes: [ ] Have you ensured that format looks appropriate for the output in which it is rendered? Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. You can merge this pull request into a Git repository by running: $ git pull https://github.com/yuri1969/nifi XSLT-securing Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi/pull/1946.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1946 commit 9469c21f3f3dc3ba137e258e3be711cb2ea5a14d Author: yuri1969 <1969yuri1969@gmail.com> Date: 2017-06-25T20:10:31Z Initial propose
            githubbot ASF GitHub Bot added a comment -

            Github user yuri1969 closed the pull request at:

            https://github.com/apache/nifi/pull/1946

            githubbot ASF GitHub Bot added a comment - Github user yuri1969 closed the pull request at: https://github.com/apache/nifi/pull/1946
            githubbot ASF GitHub Bot added a comment -

            GitHub user yuri1969 reopened a pull request:

            https://github.com/apache/nifi/pull/1946

            NIFI-4125 - Add basic security settings to TransformXml

            Thank you for submitting a contribution to Apache NiFi.

            In order to streamline the review of the contribution we ask you
            to ensure the following steps have been taken:

                1. For all changes:
            • [x] Is there a JIRA ticket associated with this PR? Is it referenced
              in the commit message?
            • [x] Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
            • [x] Has your PR been rebased against the latest commit within the target branch (typically master)?
            • [x] Is your initial contribution a single, squashed commit?
                1. For code changes:
            • [x] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder?
            • [ ] Have you written or updated unit tests to verify your changes?
            • [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
            • [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
            • [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
            • [ ] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?
                1. For documentation related changes:
            • [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
                1. Note:
                  Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.

            You can merge this pull request into a Git repository by running:

            $ git pull https://github.com/yuri1969/nifi XSLT-securing

            Alternatively you can review and apply these changes as the patch at:

            https://github.com/apache/nifi/pull/1946.patch

            To close this pull request, make a commit to your master/trunk branch
            with (at least) the following in the commit message:

            This closes #1946


            commit 9469c21f3f3dc3ba137e258e3be711cb2ea5a14d
            Author: yuri1969 <1969yuri1969@gmail.com>
            Date: 2017-06-25T20:10:31Z

            Initial propose


            githubbot ASF GitHub Bot added a comment - GitHub user yuri1969 reopened a pull request: https://github.com/apache/nifi/pull/1946 NIFI-4125 - Add basic security settings to TransformXml Thank you for submitting a contribution to Apache NiFi. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: For all changes: [x] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? [x] Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character. [x] Has your PR been rebased against the latest commit within the target branch (typically master)? [x] Is your initial contribution a single, squashed commit? For code changes: [x] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder? [ ] Have you written or updated unit tests to verify your changes? [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0] ( http://www.apache.org/legal/resolved.html#category-a)? [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly? [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly? [ ] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties? For documentation related changes: [ ] Have you ensured that format looks appropriate for the output in which it is rendered? Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. You can merge this pull request into a Git repository by running: $ git pull https://github.com/yuri1969/nifi XSLT-securing Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi/pull/1946.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1946 commit 9469c21f3f3dc3ba137e258e3be711cb2ea5a14d Author: yuri1969 <1969yuri1969@gmail.com> Date: 2017-06-25T20:10:31Z Initial propose
            githubbot ASF GitHub Bot added a comment -

            Github user mattyb149 commented on a diff in the pull request:

            https://github.com/apache/nifi/pull/1946#discussion_r124109148

            — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java —
            @@ -98,6 +99,16 @@
            .addValidator(StandardValidators.BOOLEAN_VALIDATOR)
            .build();

            + public static final PropertyDescriptor SECURE_PROCESSING = new PropertyDescriptor.Builder()
            + .name("secure-processing")
            + .displayName("Secure processing")
            + .description("Whether or not to mitigate various XML-related attacks like XXE (XML External Entity) attacks.")
            + .required(true)
            + .defaultValue("true")
            — End diff –

            Although having this default to true is probably a better idea from a security standpoint, to maintain backwards-compatibility (and not possibly disrupt existing flows), maybe this should default to false?

            githubbot ASF GitHub Bot added a comment - Github user mattyb149 commented on a diff in the pull request: https://github.com/apache/nifi/pull/1946#discussion_r124109148 — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java — @@ -98,6 +99,16 @@ .addValidator(StandardValidators.BOOLEAN_VALIDATOR) .build(); + public static final PropertyDescriptor SECURE_PROCESSING = new PropertyDescriptor.Builder() + .name("secure-processing") + .displayName("Secure processing") + .description("Whether or not to mitigate various XML-related attacks like XXE (XML External Entity) attacks.") + .required(true) + .defaultValue("true") — End diff – Although having this default to true is probably a better idea from a security standpoint, to maintain backwards-compatibility (and not possibly disrupt existing flows), maybe this should default to false?
            githubbot ASF GitHub Bot added a comment -

            Github user mattyb149 commented on a diff in the pull request:

            https://github.com/apache/nifi/pull/1946#discussion_r124109266

            — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java —
            @@ -166,8 +178,17 @@ protected PropertyDescriptor getSupportedDynamicPropertyDescriptor(final String
            .build();
            }

            • private Templates newTemplates(String path) throws TransformerConfigurationException {
              + private Templates newTemplates(ProcessContext context, String path) throws TransformerConfigurationException {
              + final Boolean secureProcessing = context.getProperty(SECURE_PROCESSING).asBoolean();
              TransformerFactory factory = TransformerFactory.newInstance();
              +
              + if (secureProcessing) {
              + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
              + // don't be overly DTD-unfriendly forcing http://apache.org/xml/features/disallow-doctype-decl
              + factory.setFeature("http://saxon.sf.net/feature/parserFeature?uri=http://xml.org/sax/features/external-parameter-entities", false);
                • End diff –

            Does this make a call out to that URL, or is that just the name of the feature?

            githubbot ASF GitHub Bot added a comment - Github user mattyb149 commented on a diff in the pull request: https://github.com/apache/nifi/pull/1946#discussion_r124109266 — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java — @@ -166,8 +178,17 @@ protected PropertyDescriptor getSupportedDynamicPropertyDescriptor(final String .build(); } private Templates newTemplates(String path) throws TransformerConfigurationException { + private Templates newTemplates(ProcessContext context, String path) throws TransformerConfigurationException { + final Boolean secureProcessing = context.getProperty(SECURE_PROCESSING).asBoolean(); TransformerFactory factory = TransformerFactory.newInstance(); + + if (secureProcessing) { + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + // don't be overly DTD-unfriendly forcing http://apache.org/xml/features/disallow-doctype-decl + factory.setFeature("http://saxon.sf.net/feature/parserFeature?uri= http://xml.org/sax/features/external-parameter-entities ", false); End diff – Does this make a call out to that URL, or is that just the name of the feature?
            githubbot ASF GitHub Bot added a comment -

            Github user yuri1969 commented on a diff in the pull request:

            https://github.com/apache/nifi/pull/1946#discussion_r124115889

            — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java —
            @@ -166,8 +178,17 @@ protected PropertyDescriptor getSupportedDynamicPropertyDescriptor(final String
            .build();
            }

            • private Templates newTemplates(String path) throws TransformerConfigurationException {
              + private Templates newTemplates(ProcessContext context, String path) throws TransformerConfigurationException {
              + final Boolean secureProcessing = context.getProperty(SECURE_PROCESSING).asBoolean();
              TransformerFactory factory = TransformerFactory.newInstance();
              +
              + if (secureProcessing) {
              + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
              + // don't be overly DTD-unfriendly forcing http://apache.org/xml/features/disallow-doctype-decl
              + factory.setFeature("http://saxon.sf.net/feature/parserFeature?uri=http://xml.org/sax/features/external-parameter-entities", false);
                • End diff –

            The URL is Saxon's (horrible) feature name.

            githubbot ASF GitHub Bot added a comment - Github user yuri1969 commented on a diff in the pull request: https://github.com/apache/nifi/pull/1946#discussion_r124115889 — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java — @@ -166,8 +178,17 @@ protected PropertyDescriptor getSupportedDynamicPropertyDescriptor(final String .build(); } private Templates newTemplates(String path) throws TransformerConfigurationException { + private Templates newTemplates(ProcessContext context, String path) throws TransformerConfigurationException { + final Boolean secureProcessing = context.getProperty(SECURE_PROCESSING).asBoolean(); TransformerFactory factory = TransformerFactory.newInstance(); + + if (secureProcessing) { + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + // don't be overly DTD-unfriendly forcing http://apache.org/xml/features/disallow-doctype-decl + factory.setFeature("http://saxon.sf.net/feature/parserFeature?uri= http://xml.org/sax/features/external-parameter-entities ", false); End diff – The URL is Saxon's (horrible) feature name.
            githubbot ASF GitHub Bot added a comment -

            Github user yuri1969 commented on a diff in the pull request:

            https://github.com/apache/nifi/pull/1946#discussion_r124116697

            — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java —
            @@ -98,6 +99,16 @@
            .addValidator(StandardValidators.BOOLEAN_VALIDATOR)
            .build();

            + public static final PropertyDescriptor SECURE_PROCESSING = new PropertyDescriptor.Builder()
            + .name("secure-processing")
            + .displayName("Secure processing")
            + .description("Whether or not to mitigate various XML-related attacks like XXE (XML External Entity) attacks.")
            + .required(true)
            + .defaultValue("true")
            — End diff –

            You are right, it alters `TransformXml` output in specific cases. TBH I don't know what is the NiFi's backwards-compatibility policy.

            githubbot ASF GitHub Bot added a comment - Github user yuri1969 commented on a diff in the pull request: https://github.com/apache/nifi/pull/1946#discussion_r124116697 — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java — @@ -98,6 +99,16 @@ .addValidator(StandardValidators.BOOLEAN_VALIDATOR) .build(); + public static final PropertyDescriptor SECURE_PROCESSING = new PropertyDescriptor.Builder() + .name("secure-processing") + .displayName("Secure processing") + .description("Whether or not to mitigate various XML-related attacks like XXE (XML External Entity) attacks.") + .required(true) + .defaultValue("true") — End diff – You are right, it alters `TransformXml` output in specific cases. TBH I don't know what is the NiFi's backwards-compatibility policy.
            githubbot ASF GitHub Bot added a comment -

            Github user mattyb149 commented on a diff in the pull request:

            https://github.com/apache/nifi/pull/1946#discussion_r124117425

            — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java —
            @@ -166,8 +178,17 @@ protected PropertyDescriptor getSupportedDynamicPropertyDescriptor(final String
            .build();
            }

            • private Templates newTemplates(String path) throws TransformerConfigurationException {
              + private Templates newTemplates(ProcessContext context, String path) throws TransformerConfigurationException {
              + final Boolean secureProcessing = context.getProperty(SECURE_PROCESSING).asBoolean();
              TransformerFactory factory = TransformerFactory.newInstance();
              +
              + if (secureProcessing) {
              + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
              + // don't be overly DTD-unfriendly forcing http://apache.org/xml/features/disallow-doctype-decl
              + factory.setFeature("http://saxon.sf.net/feature/parserFeature?uri=http://xml.org/sax/features/external-parameter-entities", false);
                • End diff –

            Ouch! Oh well, if it works it works

            githubbot ASF GitHub Bot added a comment - Github user mattyb149 commented on a diff in the pull request: https://github.com/apache/nifi/pull/1946#discussion_r124117425 — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java — @@ -166,8 +178,17 @@ protected PropertyDescriptor getSupportedDynamicPropertyDescriptor(final String .build(); } private Templates newTemplates(String path) throws TransformerConfigurationException { + private Templates newTemplates(ProcessContext context, String path) throws TransformerConfigurationException { + final Boolean secureProcessing = context.getProperty(SECURE_PROCESSING).asBoolean(); TransformerFactory factory = TransformerFactory.newInstance(); + + if (secureProcessing) { + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + // don't be overly DTD-unfriendly forcing http://apache.org/xml/features/disallow-doctype-decl + factory.setFeature("http://saxon.sf.net/feature/parserFeature?uri= http://xml.org/sax/features/external-parameter-entities ", false); End diff – Ouch! Oh well, if it works it works
            githubbot ASF GitHub Bot added a comment -

            Github user mattyb149 commented on a diff in the pull request:

            https://github.com/apache/nifi/pull/1946#discussion_r124117608

            — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java —
            @@ -98,6 +99,16 @@
            .addValidator(StandardValidators.BOOLEAN_VALIDATOR)
            .build();

            + public static final PropertyDescriptor SECURE_PROCESSING = new PropertyDescriptor.Builder()
            + .name("secure-processing")
            + .displayName("Secure processing")
            + .description("Whether or not to mitigate various XML-related attacks like XXE (XML External Entity) attacks.")
            + .required(true)
            + .defaultValue("true")
            — End diff –

            I'm not sure if there's an official backwards-compatibility policy, but I think to be safe, when adding new (optional) functionality via a processor property, the default value should match the existing behavior (in this case false).

            githubbot ASF GitHub Bot added a comment - Github user mattyb149 commented on a diff in the pull request: https://github.com/apache/nifi/pull/1946#discussion_r124117608 — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java — @@ -98,6 +99,16 @@ .addValidator(StandardValidators.BOOLEAN_VALIDATOR) .build(); + public static final PropertyDescriptor SECURE_PROCESSING = new PropertyDescriptor.Builder() + .name("secure-processing") + .displayName("Secure processing") + .description("Whether or not to mitigate various XML-related attacks like XXE (XML External Entity) attacks.") + .required(true) + .defaultValue("true") — End diff – I'm not sure if there's an official backwards-compatibility policy, but I think to be safe, when adding new (optional) functionality via a processor property, the default value should match the existing behavior (in this case false).
            githubbot ASF GitHub Bot added a comment -

            Github user yuri1969 commented on a diff in the pull request:

            https://github.com/apache/nifi/pull/1946#discussion_r124119598

            — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java —
            @@ -98,6 +99,16 @@
            .addValidator(StandardValidators.BOOLEAN_VALIDATOR)
            .build();

            + public static final PropertyDescriptor SECURE_PROCESSING = new PropertyDescriptor.Builder()
            + .name("secure-processing")
            + .displayName("Secure processing")
            + .description("Whether or not to mitigate various XML-related attacks like XXE (XML External Entity) attacks.")
            + .required(true)
            + .defaultValue("true")
            — End diff –

            Fair enough. I'll set the default to false.

            githubbot ASF GitHub Bot added a comment - Github user yuri1969 commented on a diff in the pull request: https://github.com/apache/nifi/pull/1946#discussion_r124119598 — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java — @@ -98,6 +99,16 @@ .addValidator(StandardValidators.BOOLEAN_VALIDATOR) .build(); + public static final PropertyDescriptor SECURE_PROCESSING = new PropertyDescriptor.Builder() + .name("secure-processing") + .displayName("Secure processing") + .description("Whether or not to mitigate various XML-related attacks like XXE (XML External Entity) attacks.") + .required(true) + .defaultValue("true") — End diff – Fair enough. I'll set the default to false.
            githubbot ASF GitHub Bot added a comment -

            Github user alopresto commented on a diff in the pull request:

            https://github.com/apache/nifi/pull/1946#discussion_r124423881

            — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java —
            @@ -98,6 +99,16 @@
            .addValidator(StandardValidators.BOOLEAN_VALIDATOR)
            .build();

            + public static final PropertyDescriptor SECURE_PROCESSING = new PropertyDescriptor.Builder()
            + .name("secure-processing")
            + .displayName("Secure processing")
            + .description("Whether or not to mitigate various XML-related attacks like XXE (XML External Entity) attacks.")
            + .required(true)
            + .defaultValue("false")
            — End diff –

            Is there a reason you decided to default this to `false`? Do the secure processing features add substantial time or drastically reduce the feature set of the processor?

            githubbot ASF GitHub Bot added a comment - Github user alopresto commented on a diff in the pull request: https://github.com/apache/nifi/pull/1946#discussion_r124423881 — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java — @@ -98,6 +99,16 @@ .addValidator(StandardValidators.BOOLEAN_VALIDATOR) .build(); + public static final PropertyDescriptor SECURE_PROCESSING = new PropertyDescriptor.Builder() + .name("secure-processing") + .displayName("Secure processing") + .description("Whether or not to mitigate various XML-related attacks like XXE (XML External Entity) attacks.") + .required(true) + .defaultValue("false") — End diff – Is there a reason you decided to default this to `false`? Do the secure processing features add substantial time or drastically reduce the feature set of the processor?
            githubbot ASF GitHub Bot added a comment -

            Github user mattyb149 commented on a diff in the pull request:

            https://github.com/apache/nifi/pull/1946#discussion_r124428785

            — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java —
            @@ -98,6 +99,16 @@
            .addValidator(StandardValidators.BOOLEAN_VALIDATOR)
            .build();

            + public static final PropertyDescriptor SECURE_PROCESSING = new PropertyDescriptor.Builder()
            + .name("secure-processing")
            + .displayName("Secure processing")
            + .description("Whether or not to mitigate various XML-related attacks like XXE (XML External Entity) attacks.")
            + .required(true)
            + .defaultValue("false")
            — End diff –

            That's my bad, I suggested he change it to keep the default pre-existing behavior. If true is better, I'm fine with that

            githubbot ASF GitHub Bot added a comment - Github user mattyb149 commented on a diff in the pull request: https://github.com/apache/nifi/pull/1946#discussion_r124428785 — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java — @@ -98,6 +99,16 @@ .addValidator(StandardValidators.BOOLEAN_VALIDATOR) .build(); + public static final PropertyDescriptor SECURE_PROCESSING = new PropertyDescriptor.Builder() + .name("secure-processing") + .displayName("Secure processing") + .description("Whether or not to mitigate various XML-related attacks like XXE (XML External Entity) attacks.") + .required(true) + .defaultValue("false") — End diff – That's my bad, I suggested he change it to keep the default pre-existing behavior. If true is better, I'm fine with that
            githubbot ASF GitHub Bot added a comment -

            Github user alopresto commented on a diff in the pull request:

            https://github.com/apache/nifi/pull/1946#discussion_r124429931

            — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java —
            @@ -98,6 +99,16 @@
            .addValidator(StandardValidators.BOOLEAN_VALIDATOR)
            .build();

            + public static final PropertyDescriptor SECURE_PROCESSING = new PropertyDescriptor.Builder()
            + .name("secure-processing")
            + .displayName("Secure processing")
            + .description("Whether or not to mitigate various XML-related attacks like XXE (XML External Entity) attacks.")
            + .required(true)
            + .defaultValue("false")
            — End diff –

            No worries, I found those comments in the email thread but for some reason they did not show for me in the GitHub UI. I understand the desire for backward compatibility but this is a security fix, so as long as it is well-documented in the processor and the release notes/migration notes for the release that contains this, I think `true` is a safe choice.

            githubbot ASF GitHub Bot added a comment - Github user alopresto commented on a diff in the pull request: https://github.com/apache/nifi/pull/1946#discussion_r124429931 — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java — @@ -98,6 +99,16 @@ .addValidator(StandardValidators.BOOLEAN_VALIDATOR) .build(); + public static final PropertyDescriptor SECURE_PROCESSING = new PropertyDescriptor.Builder() + .name("secure-processing") + .displayName("Secure processing") + .description("Whether or not to mitigate various XML-related attacks like XXE (XML External Entity) attacks.") + .required(true) + .defaultValue("false") — End diff – No worries, I found those comments in the email thread but for some reason they did not show for me in the GitHub UI. I understand the desire for backward compatibility but this is a security fix, so as long as it is well-documented in the processor and the release notes/migration notes for the release that contains this, I think `true` is a safe choice.
            githubbot ASF GitHub Bot added a comment -

            Github user yuri1969 commented on the issue:

            https://github.com/apache/nifi/pull/1946

            Reverted back to turned on by default.

            githubbot ASF GitHub Bot added a comment - Github user yuri1969 commented on the issue: https://github.com/apache/nifi/pull/1946 Reverted back to turned on by default.
            githubbot ASF GitHub Bot added a comment -

            Github user alopresto commented on the issue:

            https://github.com/apache/nifi/pull/1946

            Upon testing this feature, I'm not sure it's necessary. Java SE 5+ has a [restriction on entity expansion to 64,000 elements enabled by default](https://docs.oracle.com/javase/1.5.0/docs/guide/xml/jaxp/JAXP-Compatibility_150.html#JAXP_security) ([more information from Blaise Doughan here](http://blog.bdoughan.com/2011/03/preventing-entity-expansion-attacks-in.html)), so if I try to ingest the following XXE file, I get an appropriate error response:

            ```
            <!DOCTYPE foo [
            <!ENTITY a "1234567890" >
            <!ENTITY b "&a;&a;&a;&a;&a;&a;&a;&a;&a;&a;" >
            <!ENTITY c "&b;&b;&b;&b;&b;&b;&b;&b;&b;&b;" >
            <!ENTITY d "&c;&c;&c;&c;&c;&c;&c;&c;&c;&c;" >
            <!ENTITY e "&d;&d;&d;&d;&d;&d;&d;&d;&d;&d;" >
            <!ENTITY f "&e;&e;&e;&e;&e;&e;&e;&e;&e;&e;" >
            <!ENTITY g "&f;&f;&f;&f;&f;&f;&f;&f;&f;&f;" >
            <!ENTITY h "&g;&g;&g;&g;&g;&g;&g;&g;&g;&g;" >
            <!ENTITY i "&h;&h;&h;&h;&h;&h;&h;&h;&h;&h;" >
            <!ENTITY j "&i;&i;&i;&i;&i;&i;&i;&i;&i;&i;" >
            ]>
            <foo>&j;</foo>
            ```

            Error when trying to view flowfile content in "formatted" view (current master):

            `2017-06-28 15:02:40,226 ERROR [NiFi Web Server-18] o.a.nifi.web.ContentViewerController Unable to generate view of data: Unable to transform content as XML: net.sf.saxon.trans.XPathException: org.xml.sax.SAXParseException; lineNumber: 1; columnNumber: 1; JAXP00010001: The parser has encountered more than "64000" entity expansions in this document; this is the limit imposed by the JDK.`

            When routed to a `TransformXML` processor, I get an identical stacktrace in all three of the following scenarios:

            1. Current master
            2. This patch applied with "Secure processing" `false`
            3. This patch applied with "Secure processing" `true`

            Stacktrace:

            ```
            2017-06-28 15:04:49,456 ERROR [Timer-Driven Process Thread-9] o.a.n.processors.standard.TransformXml TransformXml[id=efe47d1c-015c-1000-7bae-719994808e8a] Unable to transform StandardFlowFileRecord[uuid=9f91546e-f7b4-4113-97fa-6abb1006b8c9,claim=StandardContentClaim [resourceClaim=StandardResourceClaim[id=1498687343214-1, container=default, section=1], offset=485, length=485],offset=0,name=xxe.xml,size=485] due to org.apache.nifi.processor.exception.ProcessException: IOException thrown from TransformXml[id=efe47d1c-015c-1000-7bae-719994808e8a]: java.io.IOException: net.sf.saxon.trans.XPathException: org.xml.sax.SAXParseException; lineNumber: 1; columnNumber: 1; JAXP00010001: The parser has encountered more than "64000" entity expansions in this document; this is the limit imposed by the JDK.: {}
            org.apache.nifi.processor.exception.ProcessException: IOException thrown from TransformXml[id=efe47d1c-015c-1000-7bae-719994808e8a]: java.io.IOException: net.sf.saxon.trans.XPathException: org.xml.sax.SAXParseException; lineNumber: 1; columnNumber: 1; JAXP00010001: The parser has encountered more than "64000" entity expansions in this document; this is the limit imposed by the JDK.
            at org.apache.nifi.controller.repository.StandardProcessSession.write(StandardProcessSession.java:2806)
            at org.apache.nifi.processors.standard.TransformXml.onTrigger(TransformXml.java:234)
            at org.apache.nifi.processor.AbstractProcessor.onTrigger(AbstractProcessor.java:27)
            at org.apache.nifi.controller.StandardProcessorNode.onTrigger(StandardProcessorNode.java:1120)
            at org.apache.nifi.controller.tasks.ContinuallyRunProcessorTask.call(ContinuallyRunProcessorTask.java:147)
            at org.apache.nifi.controller.tasks.ContinuallyRunProcessorTask.call(ContinuallyRunProcessorTask.java:47)
            at org.apache.nifi.controller.scheduling.TimerDrivenSchedulingAgent$1.run(TimerDrivenSchedulingAgent.java:132)
            at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
            at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308)
            at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180)
            at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294)
            at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
            at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
            at java.lang.Thread.run(Thread.java:745)
            Caused by: java.io.IOException: net.sf.saxon.trans.XPathException: org.xml.sax.SAXParseException; lineNumber: 1; columnNumber: 1; JAXP00010001: The parser has encountered more than "64000" entity expansions in this document; this is the limit imposed by the JDK.
            at org.apache.nifi.processors.standard.TransformXml$2.process(TransformXml.java:261)
            at org.apache.nifi.controller.repository.StandardProcessSession.write(StandardProcessSession.java:2785)
            ... 13 common frames omitted
            Caused by: net.sf.saxon.trans.XPathException: org.xml.sax.SAXParseException; lineNumber: 1; columnNumber: 1; JAXP00010001: The parser has encountered more than "64000" entity expansions in this document; this is the limit imposed by the JDK.
            at net.sf.saxon.event.Sender.sendSAXSource(Sender.java:460)
            at net.sf.saxon.event.Sender.send(Sender.java:171)
            at net.sf.saxon.Controller.transform(Controller.java:1692)
            at net.sf.saxon.s9api.XsltTransformer.transform(XsltTransformer.java:547)
            at net.sf.saxon.jaxp.TransformerImpl.transform(TransformerImpl.java:179)
            at org.apache.nifi.processors.standard.TransformXml$2.process(TransformXml.java:259)
            ... 14 common frames omitted
            Caused by: org.xml.sax.SAXParseException: JAXP00010001: The parser has encountered more than "64000" entity expansions in this document; this is the limit imposed by the JDK.
            at com.sun.org.apache.xerces.internal.util.ErrorHandlerWrapper.createSAXParseException(ErrorHandlerWrapper.java:203)
            at com.sun.org.apache.xerces.internal.util.ErrorHandlerWrapper.fatalError(ErrorHandlerWrapper.java:177)
            at com.sun.org.apache.xerces.internal.impl.XMLErrorReporter.reportError(XMLErrorReporter.java:400)
            at com.sun.org.apache.xerces.internal.impl.XMLErrorReporter.reportError(XMLErrorReporter.java:327)
            at com.sun.org.apache.xerces.internal.impl.XMLErrorReporter.reportError(XMLErrorReporter.java:284)
            at com.sun.org.apache.xerces.internal.impl.XMLEntityManager.startEntity(XMLEntityManager.java:1317)
            at com.sun.org.apache.xerces.internal.impl.XMLEntityManager.startEntity(XMLEntityManager.java:1241)
            at com.sun.org.apache.xerces.internal.impl.XMLDocumentFragmentScannerImpl.scanEntityReference(XMLDocumentFragmentScannerImpl.java:1902)
            at com.sun.org.apache.xerces.internal.impl.XMLDocumentFragmentScannerImpl$FragmentContentDriver.next(XMLDocumentFragmentScannerImpl.java:3058)
            at com.sun.org.apache.xerces.internal.impl.XMLDocumentScannerImpl.next(XMLDocumentScannerImpl.java:606)
            at com.sun.org.apache.xerces.internal.impl.XMLNSDocumentScannerImpl.next(XMLNSDocumentScannerImpl.java:118)
            at com.sun.org.apache.xerces.internal.impl.XMLDocumentFragmentScannerImpl.scanDocument(XMLDocumentFragmentScannerImpl.java:504)
            at com.sun.org.apache.xerces.internal.parsers.XML11Configuration.parse(XML11Configuration.java:848)
            at com.sun.org.apache.xerces.internal.parsers.XML11Configuration.parse(XML11Configuration.java:777)
            at com.sun.org.apache.xerces.internal.parsers.XMLParser.parse(XMLParser.java:141)
            at com.sun.org.apache.xerces.internal.parsers.AbstractSAXParser.parse(AbstractSAXParser.java:1213)
            at com.sun.org.apache.xerces.internal.jaxp.SAXParserImpl$JAXPSAXParser.parse(SAXParserImpl.java:643)
            at net.sf.saxon.event.Sender.sendSAXSource(Sender.java:440)
            ... 19 common frames omitted
            ```

            If this protection is enabled by the JRE, and disabling "Secure processing" doesn't affect it, what does this feature provide?

            githubbot ASF GitHub Bot added a comment - Github user alopresto commented on the issue: https://github.com/apache/nifi/pull/1946 Upon testing this feature, I'm not sure it's necessary. Java SE 5+ has a [restriction on entity expansion to 64,000 elements enabled by default] ( https://docs.oracle.com/javase/1.5.0/docs/guide/xml/jaxp/JAXP-Compatibility_150.html#JAXP_security ) ( [more information from Blaise Doughan here] ( http://blog.bdoughan.com/2011/03/preventing-entity-expansion-attacks-in.html )), so if I try to ingest the following XXE file, I get an appropriate error response: ``` <!DOCTYPE foo [ <!ENTITY a "1234567890" > <!ENTITY b "&a;&a;&a;&a;&a;&a;&a;&a;&a;&a;" > <!ENTITY c "&b;&b;&b;&b;&b;&b;&b;&b;&b;&b;" > <!ENTITY d "&c;&c;&c;&c;&c;&c;&c;&c;&c;&c;" > <!ENTITY e "&d;&d;&d;&d;&d;&d;&d;&d;&d;&d;" > <!ENTITY f "&e;&e;&e;&e;&e;&e;&e;&e;&e;&e;" > <!ENTITY g "&f;&f;&f;&f;&f;&f;&f;&f;&f;&f;" > <!ENTITY h "&g;&g;&g;&g;&g;&g;&g;&g;&g;&g;" > <!ENTITY i "&h;&h;&h;&h;&h;&h;&h;&h;&h;&h;" > <!ENTITY j "&i;&i;&i;&i;&i;&i;&i;&i;&i;&i;" > ]> <foo>&j;</foo> ``` Error when trying to view flowfile content in "formatted" view (current master): `2017-06-28 15:02:40,226 ERROR [NiFi Web Server-18] o.a.nifi.web.ContentViewerController Unable to generate view of data: Unable to transform content as XML: net.sf.saxon.trans.XPathException: org.xml.sax.SAXParseException; lineNumber: 1; columnNumber: 1; JAXP00010001: The parser has encountered more than "64000" entity expansions in this document; this is the limit imposed by the JDK.` When routed to a `TransformXML` processor, I get an identical stacktrace in all three of the following scenarios: 1. Current master 2. This patch applied with "Secure processing" `false` 3. This patch applied with "Secure processing" `true` Stacktrace: ``` 2017-06-28 15:04:49,456 ERROR [Timer-Driven Process Thread-9] o.a.n.processors.standard.TransformXml TransformXml [id=efe47d1c-015c-1000-7bae-719994808e8a] Unable to transform StandardFlowFileRecord[uuid=9f91546e-f7b4-4113-97fa-6abb1006b8c9,claim=StandardContentClaim [resourceClaim=StandardResourceClaim [id=1498687343214-1, container=default, section=1] , offset=485, length=485],offset=0,name=xxe.xml,size=485] due to org.apache.nifi.processor.exception.ProcessException: IOException thrown from TransformXml [id=efe47d1c-015c-1000-7bae-719994808e8a] : java.io.IOException: net.sf.saxon.trans.XPathException: org.xml.sax.SAXParseException; lineNumber: 1; columnNumber: 1; JAXP00010001: The parser has encountered more than "64000" entity expansions in this document; this is the limit imposed by the JDK.: {} org.apache.nifi.processor.exception.ProcessException: IOException thrown from TransformXml [id=efe47d1c-015c-1000-7bae-719994808e8a] : java.io.IOException: net.sf.saxon.trans.XPathException: org.xml.sax.SAXParseException; lineNumber: 1; columnNumber: 1; JAXP00010001: The parser has encountered more than "64000" entity expansions in this document; this is the limit imposed by the JDK. at org.apache.nifi.controller.repository.StandardProcessSession.write(StandardProcessSession.java:2806) at org.apache.nifi.processors.standard.TransformXml.onTrigger(TransformXml.java:234) at org.apache.nifi.processor.AbstractProcessor.onTrigger(AbstractProcessor.java:27) at org.apache.nifi.controller.StandardProcessorNode.onTrigger(StandardProcessorNode.java:1120) at org.apache.nifi.controller.tasks.ContinuallyRunProcessorTask.call(ContinuallyRunProcessorTask.java:147) at org.apache.nifi.controller.tasks.ContinuallyRunProcessorTask.call(ContinuallyRunProcessorTask.java:47) at org.apache.nifi.controller.scheduling.TimerDrivenSchedulingAgent$1.run(TimerDrivenSchedulingAgent.java:132) at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511) at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308) at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180) at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) at java.lang.Thread.run(Thread.java:745) Caused by: java.io.IOException: net.sf.saxon.trans.XPathException: org.xml.sax.SAXParseException; lineNumber: 1; columnNumber: 1; JAXP00010001: The parser has encountered more than "64000" entity expansions in this document; this is the limit imposed by the JDK. at org.apache.nifi.processors.standard.TransformXml$2.process(TransformXml.java:261) at org.apache.nifi.controller.repository.StandardProcessSession.write(StandardProcessSession.java:2785) ... 13 common frames omitted Caused by: net.sf.saxon.trans.XPathException: org.xml.sax.SAXParseException; lineNumber: 1; columnNumber: 1; JAXP00010001: The parser has encountered more than "64000" entity expansions in this document; this is the limit imposed by the JDK. at net.sf.saxon.event.Sender.sendSAXSource(Sender.java:460) at net.sf.saxon.event.Sender.send(Sender.java:171) at net.sf.saxon.Controller.transform(Controller.java:1692) at net.sf.saxon.s9api.XsltTransformer.transform(XsltTransformer.java:547) at net.sf.saxon.jaxp.TransformerImpl.transform(TransformerImpl.java:179) at org.apache.nifi.processors.standard.TransformXml$2.process(TransformXml.java:259) ... 14 common frames omitted Caused by: org.xml.sax.SAXParseException: JAXP00010001: The parser has encountered more than "64000" entity expansions in this document; this is the limit imposed by the JDK. at com.sun.org.apache.xerces.internal.util.ErrorHandlerWrapper.createSAXParseException(ErrorHandlerWrapper.java:203) at com.sun.org.apache.xerces.internal.util.ErrorHandlerWrapper.fatalError(ErrorHandlerWrapper.java:177) at com.sun.org.apache.xerces.internal.impl.XMLErrorReporter.reportError(XMLErrorReporter.java:400) at com.sun.org.apache.xerces.internal.impl.XMLErrorReporter.reportError(XMLErrorReporter.java:327) at com.sun.org.apache.xerces.internal.impl.XMLErrorReporter.reportError(XMLErrorReporter.java:284) at com.sun.org.apache.xerces.internal.impl.XMLEntityManager.startEntity(XMLEntityManager.java:1317) at com.sun.org.apache.xerces.internal.impl.XMLEntityManager.startEntity(XMLEntityManager.java:1241) at com.sun.org.apache.xerces.internal.impl.XMLDocumentFragmentScannerImpl.scanEntityReference(XMLDocumentFragmentScannerImpl.java:1902) at com.sun.org.apache.xerces.internal.impl.XMLDocumentFragmentScannerImpl$FragmentContentDriver.next(XMLDocumentFragmentScannerImpl.java:3058) at com.sun.org.apache.xerces.internal.impl.XMLDocumentScannerImpl.next(XMLDocumentScannerImpl.java:606) at com.sun.org.apache.xerces.internal.impl.XMLNSDocumentScannerImpl.next(XMLNSDocumentScannerImpl.java:118) at com.sun.org.apache.xerces.internal.impl.XMLDocumentFragmentScannerImpl.scanDocument(XMLDocumentFragmentScannerImpl.java:504) at com.sun.org.apache.xerces.internal.parsers.XML11Configuration.parse(XML11Configuration.java:848) at com.sun.org.apache.xerces.internal.parsers.XML11Configuration.parse(XML11Configuration.java:777) at com.sun.org.apache.xerces.internal.parsers.XMLParser.parse(XMLParser.java:141) at com.sun.org.apache.xerces.internal.parsers.AbstractSAXParser.parse(AbstractSAXParser.java:1213) at com.sun.org.apache.xerces.internal.jaxp.SAXParserImpl$JAXPSAXParser.parse(SAXParserImpl.java:643) at net.sf.saxon.event.Sender.sendSAXSource(Sender.java:440) ... 19 common frames omitted ``` If this protection is enabled by the JRE, and disabling "Secure processing" doesn't affect it, what does this feature provide?
            githubbot ASF GitHub Bot added a comment -

            Github user yuri1969 commented on the issue:

            https://github.com/apache/nifi/pull/1946

            The `transformXml` uses Saxon HE implementation of a XSLT Processor. So Saxon implements the features.

            • `FEATURE_SECURE_PROCESSING` is XSLT Processor feature that restricts XSLT functionality like `system-property()` to access Java system properties, using relative URIs in `xsl:result-document`, etc. So it should mitigate some threats coming from using non-trusted XSLTs.
            • both `http://xml.org/sax/features/external` are XML Parser features that restrict using `<!ENTITY` of `<!DOCTYPE` in a XML file. An attacker can use entities to obtain access to your FS.

            For example a malicious non-trusted input XML:
            ```
            <?xml version="1.0" encoding="utf-8"?>
            <!DOCTYPE foo [
            <!ELEMENT foo ANY >
            <!ENTITY xxe SYSTEM "file:///etc/passwd" >]>
            <foo>&xxe;</foo>
            ```
            Then a XSLT output containing element `foo` leaks content of your /etc/passwd file.

            • Billion laughs/LOL bomb/Entity expansion DoS is trully secured by JRE default as you showed. So no need for a feature here.
            githubbot ASF GitHub Bot added a comment - Github user yuri1969 commented on the issue: https://github.com/apache/nifi/pull/1946 The `transformXml` uses Saxon HE implementation of a XSLT Processor. So Saxon implements the features. `FEATURE_SECURE_PROCESSING` is XSLT Processor feature that restricts XSLT functionality like `system-property()` to access Java system properties, using relative URIs in `xsl:result-document`, etc. So it should mitigate some threats coming from using non-trusted XSLTs. both ` http://xml.org/sax/features/external ` are XML Parser features that restrict using `<!ENTITY` of `<!DOCTYPE` in a XML file. An attacker can use entities to obtain access to your FS. For example a malicious non-trusted input XML: ``` <?xml version="1.0" encoding="utf-8"?> <!DOCTYPE foo [ <!ELEMENT foo ANY > <!ENTITY xxe SYSTEM "file:///etc/passwd" >]> <foo>&xxe;</foo> ``` Then a XSLT output containing element `foo` leaks content of your /etc/passwd file. Billion laughs/LOL bomb/Entity expansion DoS is trully secured by JRE default as you showed. So no need for a feature here.

            Commit 3bf1d127062a2d52d7be32e5ef29e19242219f48 in nifi's branch refs/heads/master from yuri1969
            [ https://git-wip-us.apache.org/repos/asf?p=nifi.git;h=3bf1d12 ]

            NIFI-4125 Added secure transform feature and configuration to TransformXML processor to mitigate XXE file system leaks.

            This closes #1946.

            Signed-off-by: Andy LoPresto <alopresto@apache.org>

            jira-bot ASF subversion and git services added a comment - Commit 3bf1d127062a2d52d7be32e5ef29e19242219f48 in nifi's branch refs/heads/master from yuri1969 [ https://git-wip-us.apache.org/repos/asf?p=nifi.git;h=3bf1d12 ] NIFI-4125 Added secure transform feature and configuration to TransformXML processor to mitigate XXE file system leaks. This closes #1946. Signed-off-by: Andy LoPresto <alopresto@apache.org>
            githubbot ASF GitHub Bot added a comment -

            Github user alopresto commented on the issue:

            https://github.com/apache/nifi/pull/1946

            Thanks for that detailed explanation. I updated the commit message to fit our format.

            Ran `contrib-check` and all tests pass. +1, merging.

            githubbot ASF GitHub Bot added a comment - Github user alopresto commented on the issue: https://github.com/apache/nifi/pull/1946 Thanks for that detailed explanation. I updated the commit message to fit our format. Ran `contrib-check` and all tests pass. +1, merging.
            githubbot ASF GitHub Bot added a comment -

            Github user asfgit closed the pull request at:

            https://github.com/apache/nifi/pull/1946

            githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/nifi/pull/1946

            People

              Unassigned Unassigned
              yuri1969 Yuri
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: