per SA-CORE-2009-007

In certain cases, the user password can be exposed in the REQUEST values

D6 patch:

Index: includes/pager.inc
===================================================================
RCS file: /cvs/drupal/drupal/includes/pager.inc,v
retrieving revision 1.63
diff -u -p -r1.63 pager.inc
--- includes/pager.inc	6 Dec 2007 09:58:30 -0000	1.63
+++ includes/pager.inc	1 Jun 2009 10:18:50 -0000
@@ -85,7 +85,7 @@ function pager_query($query, $limit = 10
 function pager_get_querystring() {
   static $string = NULL;
   if (!isset($string)) {
-    $string = drupal_query_string_encode($_REQUEST, array_merge(array('q', 'page'), array_keys($_COOKIE)));
+    $string = drupal_query_string_encode($_REQUEST, array_merge(array('q', 'page', 'pass'), array_keys($_COOKIE)));
   }
   return $string;
 }
Index: includes/tablesort.inc
===================================================================
RCS file: /cvs/drupal/drupal/includes/tablesort.inc,v
retrieving revision 1.47
diff -u -p -r1.47 tablesort.inc
--- includes/tablesort.inc	4 Jan 2008 09:31:48 -0000	1.47
+++ includes/tablesort.inc	1 Jun 2009 10:18:50 -0000
@@ -136,7 +136,7 @@ function tablesort_cell($cell, $header, 
  *   except for those pertaining to table sorting.
  */
 function tablesort_get_querystring() {
-  return drupal_query_string_encode($_REQUEST, array_merge(array('q', 'sort', 'order'), array_keys($_COOKIE)));
+  return drupal_query_string_encode($_REQUEST, array_merge(array('q', 'sort', 'order', 'pass'), array_keys($_COOKIE)));
 }
 
 /**

Comments

Anonymous’s picture

Status: Patch (to be ported) » Needs review
StatusFileSize
new1.36 KB

Status: Needs review » Needs work

The last submitted patch failed testing.

brianV’s picture

Status: Needs work » Needs review
StatusFileSize
new1.43 KB

Re-rolled.

damien tournoud’s picture

As discussed internally to the security team while we were fixing this issue, the correct fix for D7 is to suppress this silly and dangerous "copy POST values to GET parameters" behavior.

damien tournoud’s picture

StatusFileSize
new707 bytes

This should do it (and points us to pages in core that use the broken behavior, if we have some).

damien tournoud’s picture

Component: forum.module » base system

Reassigning properly.

damien tournoud’s picture

StatusFileSize
new1.23 KB

And we need to remove that broken feature from tablesort too (hat tip from brianV).

brianV’s picture

Status: Needs review » Reviewed & tested by the community

All green, looks good here.

dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

yonailo’s picture

Hello,

Le Figaro.fr has had a problem with D6 that is related to this issue:
http://plus.lefigaro.fr/note/faille-de-securite-mon-figaro-les-explicati...

Do you think that the patch applied in D6 is enough ?
Is it possible to apply the solution adopted for D7 directly in D6 ?

As D. Tournoud said in #4, I would also like to suppress this silly and dangerous "copy POST values to GET parameters" behavior." ....also in D6... why not ?

Thank you in advance.