Drupal 4.6.4 encodes invalid arg separator?

CoR - December 1, 2005 - 15:20
Project:Drupal
Version:4.6.4
Component:base system
Category:bug report
Priority:critical
Assigned:chx
Status:closed
Description

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.

#1

lennart - December 1, 2005 - 15:26
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.

#2

CoR - December 1, 2005 - 15:47

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?

#3

chx - December 1, 2005 - 15:49

merge: set one of the reports to duplicate

#4

lennart - December 1, 2005 - 16:02

I have set the other post to duplicate

#5

lennart - December 1, 2005 - 18:16

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

#6

CoR - December 1, 2005 - 20:07

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

#7

Psicomante - December 1, 2005 - 23:26

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

#8

chx - December 2, 2005 - 01:20
Assigned to:Anonymous» chx

psicomante that's VERY helpful, thanks.

#9

chx - December 2, 2005 - 01:26
Status:active» patch (code needs review)

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.

AttachmentSize
check_url.patch525 bytes

#10

webchick - December 2, 2005 - 01:51
Status:patch (code needs review)» patch (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.

#11

lennart - December 2, 2005 - 02:04

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

#12

chx - December 2, 2005 - 02:09

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.

#13

Amazon - December 2, 2005 - 02:16

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

#14

Pogo - December 2, 2005 - 10:19

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?

#15

Pogo - December 2, 2005 - 10:21

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

#16

chx - December 2, 2005 - 10:26

you definitely should

#17

Dries - December 2, 2005 - 10:31

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

#18

Pogo - December 2, 2005 - 10:47

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.

#19

Psicomante - December 2, 2005 - 13:06

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

OSS Rocks!

#20

webchick - December 3, 2005 - 01:32
Status:patch (reviewed & tested by the community)» fixed

Setting this to fixed, since it was committed.

#21

meba - December 5, 2005 - 08:52

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

#22

webchick - December 5, 2005 - 13:52

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.

#23

markus_petrux - December 7, 2005 - 16:35

Oh, ah!

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

#24

Anonymous - December 21, 2005 - 16:41
Status:fixed» closed
 
 

Drupal is a registered trademark of Dries Buytaert.