Tests for access bypass in upload module

pwolanin - October 10, 2008 - 00:37
Project:Drupal
Version:7.x-dev
Component:upload.module
Category:bug report
Priority:critical
Assigned:Unassigned
Status:won't fix
Description

reported by ccode with suggested fix to the security team for 6.x, fix in SA-2008-60 for 6.x with patch by Gabor.

In upload.module, near the top of function upload_node_form_submit(),
there is a line,

if (($user->uid != 1 || user_access('upload files')) && ...

The "!=" should be "==" like this,

if (($user->uid == 1 || user_access('upload files')) && ...

meaning you are the admin or you have access privilege.

Looking into user_access(), you see admin has full access rights. So,
really, the if statement should be,

if (user_access('upload files') && ...

#1

pwolanin - October 10, 2008 - 00:45
Status:active» needs review

here's the change that was committed to 6.x.

AttachmentSizeStatusTest resultOperations
upload_bypass-319328.patch939 bytesIgnoredNoneNone

#2

drewish - October 10, 2008 - 00:47

seems like a good thing to have a test for...

#3

pwolanin - October 10, 2008 - 00:49

ccode is: http://drupal.org/user/306203

@drewish - sure, but we should get these security patches that went in 6.x into 7.x asap and can revisit the need for new tests. Otherwise they are likely to be forgotten, no longer apply, etc.

#4

earnie - October 10, 2008 - 11:57
Status:needs review» reviewed & tested by the community

Yes, this makes sense to me.

EDIT: Please commit and then set as CNW for the test.

#5

webchick - October 11, 2008 - 02:58
Status:reviewed & tested by the community» needs work

Committed. Marking CNW for test. ;)

#6

catch - October 11, 2008 - 09:40
Title:access bypass in upload module» Tests for access bypass in upload module
Component:upload.module» tests

Moving around so we know the original patch went in easier.

#7

catch - October 11, 2008 - 09:40
Status:needs work» active

#8

grendzy - October 11, 2009 - 18:37
Component:tests» base system
Status:active» postponed

see #563000: Remove upload module and provide upgrade path to file

#9

grendzy - October 11, 2009 - 18:38
Component:base system» upload.module

#10

webchick - December 6, 2009 - 00:55
Status:postponed» won't fix

Whelp. Guess we don't need this anymore. ;)

 
 

Drupal is a registered trademark of Dries Buytaert.