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') && ...
CommentFileSizeAuthor
#1 upload_bypass-319328.patch939 bytespwolanin

Comments

pwolanin’s picture

pwolanin commented
Status: Active » Needs review
StatusFileSize
new upload_bypass-319328.patch939 bytes

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

drewish’s picture

drewish commented

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

pwolanin’s picture

pwolanin commented

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.

Anonymous’s picture

Anonymous (not verified) commented
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.

webchick’s picture

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

Committed. Marking CNW for test. ;)

catch’s picture

catch
he/him
Primary language English
commented
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.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Active
grendzy’s picture

grendzy commented
Component: tests » base system
Status: Active » Postponed

see #563000: Provide upgrade path to file

grendzy’s picture

grendzy commented
Component: base system » upload.module
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Postponed » Closed (won't fix)

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