http://drupal.org/SA-CORE-2012-004

CommentFileSizeAuthor
#17 file-munge-filename-1870612-17.patch825 bytesplach
#17 file-munge-filename-1870612-17-WITH-ROLLBACK.patch1.43 KBplach
#11 file-munge-filename-1870612-11-WITH-ROLLBACK.patch1.56 KBDavid_Rothstein
#11 file-munge-filename-1870612-11.patch933 bytesDavid_Rothstein
#9 file-munge-filename-1870612-9-WITH-ROLLBACK.patch1.55 KBDavid_Rothstein
#9 file-munge-filename-1870612-9.patch931 bytesDavid_Rothstein
57404_null_byte_file_munge_filename_17-D8.patch661 bytesgreggles

Comments

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Priority: Normal » Critical
Issue tags: +Security Advisory follow-up

Well, probably some other metadata is important.

Status: Reviewed & tested by the community » Needs work
Issue tags: -Security Advisory follow-up

The last submitted patch, 57404_null_byte_file_munge_filename_17-D8.patch, failed testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs work » Needs review
Issue tags: +Security Advisory follow-up

57404_null_byte_file_munge_filename_17-D8.patch queued for re-testing.

David_Rothstein’s picture

David_Rothstein commented

Also tagging as a release blocker for the next D7 release (just in case it turns out there's anything in the latest 7.x-dev code we need to do as followup for this, although I doubt it).

Status: Needs review » Needs work
Issue tags: -Security Advisory follow-up

The last submitted patch, 57404_null_byte_file_munge_filename_17-D8.patch, failed testing.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Status: Needs work » Needs review
Issue tags: +Security Advisory follow-up

57404_null_byte_file_munge_filename_17-D8.patch queued for re-testing.

Tests pass here.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Status: Needs review » Reviewed & tested by the community

This is a straight port of the D7 patch. Tests pass, angels sing. RTBC :)

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Title: SA-CORE-2012-004 - Drupal core - Arbitrary code execution via file upload » Tests for SA-CORE-2012-004 - Drupal core - Arbitrary code execution via file upload
Component: upload.module » file system
Category: bug » task
Priority: Critical » Major
Status: Reviewed & tested by the community » Active
Issue tags: +Needs tests, +Needs backport to D7

Committed and pushed to 8.x, but we need test coverage for this.

David_Rothstein’s picture

David_Rothstein commented
Status: Active » Needs review
StatusFileSize
new file-munge-filename-1870612-9.patch931 bytes
new file-munge-filename-1870612-9-WITH-ROLLBACK.patch1.55 KB

Maybe just something like this?

Status: Needs review » Needs work

The last submitted patch, file-munge-filename-1870612-9.patch, failed testing.

David_Rothstein’s picture

David_Rothstein commented
Status: Needs work » Needs review
StatusFileSize
new file-munge-filename-1870612-11.patch933 bytes
new file-munge-filename-1870612-11-WITH-ROLLBACK.patch1.56 KB

Hm, I don't claim to have tested those myself actually, but I thought I at least ran them through php -l to check for syntax errors. Apparently not :)

These should be better.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Status: Needs review » Reviewed & tested by the community

The test looks good to me, but what about appending it to another test method to save a new drupal installation and speed things up a bit?

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Status: Reviewed & tested by the community » Needs review

Didn't mean to change the status (yet :).

David_Rothstein’s picture

David_Rothstein commented

Well, that entire file basically uses the one-test-per-method pattern already, so I didn't want to break the pattern here.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Status: Needs review » Reviewed & tested by the community

Sound good.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Lovely. Thanks, David!

Committed and pushed to 8.x.

Those tests should be backported to 7.x too, methinks.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new file-munge-filename-1870612-17-WITH-ROLLBACK.patch1.43 KB
new file-munge-filename-1870612-17.patch825 bytes

Straight reroll.

Status: Needs review » Needs work

The last submitted patch, file-munge-filename-1870612-17-WITH-ROLLBACK.patch, failed testing.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Status: Needs work » Reviewed & tested by the community

Cool

David_Rothstein’s picture

David_Rothstein commented
Status: Reviewed & tested by the community » Fixed

Committed to 7.x - http://drupalcode.org/project/drupal.git/commit/aecf889

David_Rothstein’s picture

David_Rothstein commented
Title: Tests for SA-CORE-2012-004 - Drupal core - Arbitrary code execution via file upload » SA-CORE-2012-004 - Drupal core - Arbitrary code execution via file upload
Category: task » bug
Priority: Major » Critical

I'm also 100% convinced that there's nothing left to do on 7.x-dev for this issue, so removing tag.

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