Problem/Motivation

Some files from module shows the error "unserialize() is insecure unless allowed classes are limited. Use a safe format like JSON or use the allowed_classes option." when verifying code standards with PHPCS.

Steps to reproduce

Run the following command at module folder:
phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,md,yml *

Proposed resolution

Change the code to use the allowed_classes option.

Issue fork webform-3318334

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

adaucyj created an issue. See original summary.

adaucyj opened merge request !242

adaucyj’s picture

adaucyj
He/Him
Primary language Portuguese, Brazil
Location Irupi, ES
CreditAttribution: adaucyj at CI&T commented
Assigned: adaucyj » Unassigned
Status: Active » Needs review

Could someone review it, please.

erikaagp’s picture

erikaagp CreditAttribution: erikaagp at CI&T commented
Assigned: Unassigned » erikaagp

I'll review it

cilefen’s picture

cilefen CreditAttribution: cilefen as a volunteer commented
Status: Needs review » Needs work
Issue tags: +Security improvements

I think there is a base branch issue with this merge request because it changes 360 files! It needs that fixed before it can be reviewed.

adaucyj closed merge request !242

adaucyj opened merge request !243

adaucyj’s picture

adaucyj
He/Him
Primary language Portuguese, Brazil
Location Irupi, ES
CreditAttribution: adaucyj at CI&T commented
Status: Needs work » Needs review

Hello, @cilefen. Thanks for notice that. I created a new MR, now pointing to the right branch and having only the commit regarding to this issue.

Could someone review it, please?

erikaagp’s picture

erikaagp CreditAttribution: erikaagp at CI&T commented
Assigned: erikaagp » Unassigned
Status: Needs review » Reviewed & tested by the community

Great! I didn't find any phpcs error related with "Error: unserialize() is insecure". So I think I can move it to RTBC. There are other phpcs errors, but none of them is related with this.

jrockowitz’s picture

jrockowitz CreditAttribution: jrockowitz as a volunteer and at Webform module Open Collective, The Big Blue House commented

Where does Drupal core stand on this issue?

There are insecure instances of unserialize() in Drupal core.

Shouldn't this be fixed in 6.1.x?

  • 933c47c committed on 6.1.x 
    Issue #3318334 by adaucyj: Error: unserialize() is insecure

  • 933c47c committed on 6.x 
    Issue #3318334 by adaucyj: Error: unserialize() is insecure

  • 933c47c committed on 6.2.x 
    Issue #3318334 by adaucyj: Error: unserialize() is insecure
jrockowitz’s picture

jrockowitz CreditAttribution: jrockowitz as a volunteer and at Webform module Open Collective, The Big Blue House commented
Status: Reviewed & tested by the community » Fixed

I am seeing this a lot in contrib module. I am just going to commit the patch.

Status: Fixed » Closed (fixed)

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