Download & Extend

Tests for access bypass in upload module

Project:Drupal core
Version:7.x-dev
Component:upload.module
Category:bug report
Priority:critical
Assigned:Unassigned
Status:closed (won't fix)

Issue Summary

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') && ...

Comments

#1

Status:active» needs review

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

AttachmentSizeStatusTest resultOperations
upload_bypass-319328.patch939 bytesIgnored: Check issue status.NoneNone

#2

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

#3

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

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

Status:reviewed & tested by the community» needs work

Committed. Marking CNW for test. ;)

#6

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

Status:needs work» active

#8

Component:tests» base system
Status:active» postponed

see #563000: Provide upgrade path to file

#9

Component:base system» upload.module

#10

Status:postponed» closed (won't fix)

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

nobody click here