RHEL Engineering is moving the tracking of its product development work on RHEL 6 through RHEL 9 to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "RHEL project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs in the statuses "NEW", "ASSIGNED", and "POST" are being migrated throughout September 2023. Bugs of Red Hat partners with an assigned Engineering Partner Manager (EPM) are migrated in late September as per pre-agreed dates. Bugs against components "kernel", "kernel-rt", and "kpatch" are only migrated if still in "NEW" or "ASSIGNED". If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "RHEL project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/RHEL-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.
Bug 855117 - ccs: harmful handling of XML entities
Summary: ccs: harmful handling of XML entities
Keywords:
Status: CLOSED DUPLICATE of bug 855112
Alias: None
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: ricci
Version: 6.3
Hardware: Unspecified
OS: Unspecified
medium
medium
Target Milestone: rc
: ---
Assignee: Chris Feist
QA Contact: Cluster QE
URL:
Whiteboard:
: 987765 (view as bug list)
Depends On: 855112 855121
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-09-06 18:10 UTC by Jan Pokorný [poki]
Modified: 2019-04-16 14:00 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of: 855112
Environment:
Last Closed: 2014-03-27 16:44:42 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Bugzilla 739600 1 None None None 2021-01-20 06:05:38 UTC

Internal Links: 739600

Description Jan Pokorný [poki] 2012-09-06 18:10:37 UTC
+++ This bug was initially created as a clone of Bug #855112 +++

In connection to case discussed in the public cluster ML [1], we need
to make sure the user input that makes its way into XML (either as
cluster.conf or XML-formatted requests towards ricci) is XML-ready.
That is, the "unsafe" characters are encoded as XML entities.

Otherwise, such requests have no effect and unfortunately, neither
ricci provides a sensible diagnostics about this.  Example of such
input is the password for the initial authentization against ricci.

[1] https://www.redhat.com/archives/linux-cluster/2012-August/msg00135.html

Comment 1 Jan Pokorný [poki] 2012-09-07 23:24:32 UTC
I proposed a (roughly tested) patch in (temporary) branch upstream:
http://git.fedorahosted.org/cgit/conga.git/commit/?h=bz855117

Comment 3 Jan Pokorný [poki] 2012-09-11 13:54:42 UTC
Better patch pushed to the referenced branch (previous version branch
removed).

Sidenote: there is no need to preserve existing & etc. when
escaping the values as when such entity is present, it means just
this sequence of characters (so & should get escaped as well).
Stating this, as it used to be a behavior in ricci, but
it is to be removed as well [bug 855121].

Comment 4 Jan Pokorný [poki] 2012-09-13 16:10:15 UTC
Previous solutions were over-approximations of the correct fix.

The referenced branch should now contain the Right Thing.
Only the password needs to be escaped (postponed for when really
needed), other variables in the resulting request XML are set
directly in to the XML object which gets this right upon its
serialization (toxml()).

Comment 5 Chris Feist 2012-09-17 23:00:30 UTC
You should only be changing code in the check_authentication function all of the other changes could potentially affect other parts of the code, this patch should only fix issues from this bz.

Also, it looks like there is no check for '<', '>' and '&'.  Those need to be added to the escape.

Comment 6 Jan Pokorný [poki] 2012-09-18 10:57:21 UTC
re [comment 5]

> You should only be changing code in the check_authentication function
> all of the other changes could potentially affect other parts of
> the code, this patch should only fix issues from this bz.

Well, beside the main change, I made a (very conservative -- only around
the parts being changed) alignment of code closer to the better shape/less
debt (unused imports, more natural indentation).  It's unlikely this could
cause any issue and also unlikely such debt could be diminished by other
means.  Unfortunately, the unit test did not work for me, both before
and after the change.

> Also, it looks like there is no check for '<', '>' and '&'.  Those need
> to be added to the escape.

They indeed are handled in escape function, implicitly by its definition.
The explicit ones are added so the complete set is treated the same way,
just as what libxml2 does with attribute values automatically (so all
the communicating sides has unified approach).

Comment 7 Chris Feist 2012-09-18 13:48:27 UTC
Ahh, just double checked that escape function, ok we should be good to go with that.  Just remove all the extra unrelated changes and I'll pull the patch in.

Thanks,
Chris

Comment 8 Jan Pokorný [poki] 2012-09-26 21:34:23 UTC
Ok, unrelated stuff has to wait for other occasion (will post a separate
patch), but now when ricci protocol bump seems to happen, it makes sense
to read such version sent by ricci properly, adjust single/double escaping
of password respectively and also to issue a warning about recommended
update of ricci+modcluster (unfortunately, some refactoring was needed
so send_to_ricci depends on already initialized secure socket it gets
as a parameter).

I refreshed the referred branch with such updated changeset.
Does it make sense?

Comment 13 Jan Pokorný [poki] 2013-08-12 14:17:43 UTC
*** Bug 987765 has been marked as a duplicate of this bug. ***

Comment 15 Fabio Massimo Di Nitto 2014-03-27 16:44:42 UTC

*** This bug has been marked as a duplicate of bug 855112 ***


Note You need to log in before you can comment on or make changes to this bug.