Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid XML attributes due to characters not escaped #520

Open
jer-sen opened this issue Jul 10, 2023 · 2 comments
Open

Invalid XML attributes due to characters not escaped #520

jer-sen opened this issue Jul 10, 2023 · 2 comments

Comments

@jer-sen
Copy link

jer-sen commented Jul 10, 2023

For example, a & in ACS URL is not replaced by & in the loginRequestRedirectURL function so the generated XML is not valid and authentication fails.
More important, this could lead to security issues (XML injection).

Solution: do NOT use replaceTagsByValue in an XML template without escaping the values!

@tngan
Copy link
Owner

tngan commented Jul 24, 2023

@jer-sen replaceTagsByValue is definitely not the optimal function to construct xml, it's doing string interpolation. I will draft a new function and add a deprecation warning for this function.

@Munawwar
Copy link
Contributor

Munawwar commented Aug 16, 2023

@tngan I have a fix at https://github.com/Carriyo/samlify/tree/xml-entity-escape (though tests werent working for me. There is an issue with test setup, which is topic for another discussion. once tests are running, tests pass.)

The assumption is that if an interpolation begins with a double quote (e.g. "{ID}) then its an interpolation within an XML attribute that needs character entity escaping. This assumption seems to hold for your current templates.

Raised a PR: #523. I haven't followed all the repo conventions yet. I could do those as another commit on the branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants