The latest Durpal 4.6.4 upgrade causes invalid arg separator encoding, namely the '&' appears in any internal URLs like column sorting, and there should be only '&'. This appears to be module-independent.

Each additional click on the "offensive" url recodes '&' into '&amp', and '&amp' etc...

I am not that familiar with internal Drupal workings to be able to track this myself.

I've noticed the problem after upgrading from 4.6.3 to 4.6.4. I replaced only the files in includes/ and modules/ directories. In addition to core modules, I have flexinode, gsitemap and poormanscron installed which I did not update. I did not launch update.php as the security notice said there were no changes in API or database.

CommentFileSizeAuthor
#9 check_url.patch525 byteschx
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

lennart’s picture

lennart CreditAttribution: lennart commented
Priority: Normal » Critical

yes - there is a problem with that. I think it is the same issue described here
http://drupal.org/node/39565
but your description is more precise, I think.

CoR-1’s picture

CoR-1 CreditAttribution: CoR-1 commented

Heh, it is the same, yes. We were apparently posting at the same time, as that post did not come up when I searched for it, before I submitted this.

That other post also describes the doubling of any variables that come after failed arg separator, but invalidly url_encoded. This I noticed as well.

Can these two reports be merged?

chx’s picture

chx CreditAttribution: chx commented

merge: set one of the reports to duplicate

lennart’s picture

lennart CreditAttribution: lennart commented

I have set the other post to duplicate

lennart’s picture

lennart CreditAttribution: lennart commented

does anyone know which include this stems from ? I tried replacing the common.inc with an earlier one, but it broke my sites. I think this problem is rather critical as it affects many modules. 4.6.4 is for production and many production sites are now malfunctioning

CoR-1’s picture

CoR-1 CreditAttribution: CoR-1 commented

I think XSS patching did something wrong. I think the problem is in filter.module, or anything else related to XSS.

psicomante’s picture

psicomante CreditAttribution: psicomante commented

it happens in any link with "&", not only tables.

chx’s picture

chx CreditAttribution: chx commented
Assigned: Unassigned » chx

psicomante that's VERY helpful, thanks.

chx’s picture

chx CreditAttribution: chx commented
Status: Active » Needs review
FileSize
check_url.patch525 bytes

Here's what happens. First we do something which (check the code) equals to a check_plain call. Next, we call filter_xss_bad_protocol. Look at the last line of that function... there's the double escape.

I think this is a version agnostic problem.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Needs review » Reviewed & tested by the community

Confirmed the problem in 4.6.4 by going to adminster >> content. Clicking on any of the pagination links at the bottom resulted in double-escaped &, with the effect being that the page would reload but the table contents would remain the same.

This patch solved the problem perfectly.

Setting ready for commit.

lennart’s picture

lennart CreditAttribution: lennart commented

I can confirm that this patch works! Thanks CHX :)

chx’s picture

chx CreditAttribution: chx commented

we have learned something extremely important here: a good bug report equals the solution. I was aware of this problem for 12+ hrs and was reluctant to look into it. But when it was reported that every url containg the ampersand is affected, I immediately knew what's the problem.

Amazon’s picture

Amazon CreditAttribution: Amazon commented

I applied to this page to Drupal 4.6.4 tar ball.

I added 12 pages.

I went to the home page.

I clicked back and forth on the pagination and it works.

Kieran

Pogo’s picture

Pogo CreditAttribution: Pogo commented

The patch fixes all link problems but aggregator ones (links to external sites). These links are already saved in DB with '&' instead of '&'. Should I file a bug report in aggregator.module?

Pogo’s picture

Pogo CreditAttribution: Pogo commented

The first '&' was '& a m p ;'. Drupal somehow decoded the entity, another bug? :-)

chx’s picture

chx CreditAttribution: chx commented

you definitely should

Dries’s picture

Dries CreditAttribution: Dries commented

Committed to DRUPAL-4-5, DRUPAL-4-6 and HEAD.

Pogo’s picture

Pogo CreditAttribution: Pogo commented

Critical aggregator.module bug caused by XSS filtering patch was filed as http://drupal.org/node/39670 . This is 4.6.4 regression against 4.6.3.

psicomante’s picture

psicomante CreditAttribution: psicomante commented

I'm very glad to help this glorious community! thanks CHX ;)

OSS Rocks!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Setting this to fixed, since it was committed.

meba’s picture

meba CreditAttribution: meba commented

Are you sure, that this problem is fixed in Drupal 4.6.4 tar.gz distribution? More users are still describing this problem...

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented

No, fixes don't go into a release once it's been packaged. It's fixed in the 4.6 branch of CVS. Once a few more 4.6.4-related bugs are fixed, a 4.6.5 (or 4.6.4.1) will likely be released.

markus_petrux’s picture

markus_petrux CreditAttribution: markus_petrux commented

Oh, ah!

I saw this reported here first:
http://drupal.org/node/40094

Anonymous’s picture

(not verified) CreditAttribution: commented
Status: Fixed » Closed (fixed)