WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#42431 closed defect (bug) (fixed)

wpdb prepare - {} replaced with % if AUTH_SALT is defined as null or empty string

Reported by: jsonfry Owned by: pento
Milestone: 4.9 Priority: normal
Severity: normal Version: 4.8.3
Component: Database Keywords: has-patch commit dev-reviewed fixed-major
Focuses: Cc:
PR Number:

Description

In includes/wp-db.php line 1967, the defined function is used which checks if the constant has been set, but not if it's actually got a (usable) value in it. It could also also check for null / or empty string.

This manifested for us when adding / removing a user role. In our roles we have a user that has no capabilities, and when we added a new role after updating to 4.8.3 the php serialzed array that gets saved to wp_user_roles in the wp_options table has a % instead of a {}, which was pretty fatal when attempting to deserialize it - our site then appeared to have no roles.

(We should have had AUTH_SALT set, but we didn't. We use Bedrock so it was expecting AUTH_SALT as an env var, and setting is regardless.

Attachments (2)

auth-salt-fallback.diff (600 bytes) - added by jsonfry 2 years ago.
patch for the 4.8 branch
42431.diff (647 bytes) - added by pento 2 years ago.

Download all attachments as: .zip

Change History (23)

#1 @jsonfry
2 years ago

  • Focuses administration removed

#2 @jsonfry
2 years ago

Also, the fallback to rand() doesn't work with type checking either as the 3rd param for hash_hmac should be a string, not an int.

@jsonfry
2 years ago

patch for the 4.8 branch

#3 @pento
2 years ago

  • Milestone changed from Awaiting Review to 4.9
  • Owner set to pento
  • Status changed from new to assigned

Thank you for the patch, @jsonfry! We addressed the string cast in in #42401, but you're right that we should also check that AUTH_SALT has a value.

@pento
2 years ago

#4 @pento
2 years ago

  • Keywords has-patch dev-feedback commit added

42431.diff updates the patch to apply cleanly against trunk.

This ticket was mentioned in Slack in #core by pento. View the logs.


2 years ago

This ticket was mentioned in Slack in #core-committers by melchoyce. View the logs.


2 years ago

#7 @aaroncampbell
2 years ago

  • Keywords dev-reviewed added; dev-feedback removed

Looks good and works as I would expect.

#8 @pento
2 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 42120:

WPDB: Check that AUTH_SALT is not empty.

In wpdb::placeholder_escape(), the key for hash_hmac() defaults to AUTH_SALT, but hash_hmac() will return an empty string if the key is empty.

This had the side effect of the string {} being incorrectly replaced with a % character in queries just about to be run on the database.

Props jsonfry.
Fixes #42431.

#9 @pento
2 years ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Re-opening for backporting to the 3.7-4.8 branches.

#10 @dd32
2 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 42230:

WPDB: Check that AUTH_SALT is not empty, Fix a PHP notice when AUTH_SALT is undefined.

Props jsonfry, mkomar, pento.
Merges [42119] and [42120] to the 4.8 branch.
Fixes #42431 and #42401 for 4.8.

#11 @dd32
2 years ago

In 42231:

WPDB: Check that AUTH_SALT is not empty, Fix a PHP notice when AUTH_SALT is undefined.

Props jsonfry, mkomar, pento.
Merges [42119] and [42120] to the 4.7 branch.
Fixes #42431 and #42401 for 4.7.

#12 @dd32
2 years ago

In 42232:

WPDB: Check that AUTH_SALT is not empty, Fix a PHP notice when AUTH_SALT is undefined.

Props jsonfry, mkomar, pento.
Merges [42119] and [42120] to the 4.6 branch.
Fixes #42431 and #42401 for 4.6.

#13 @dd32
2 years ago

In 42233:

WPDB: Check that AUTH_SALT is not empty, Fix a PHP notice when AUTH_SALT is undefined.

Props jsonfry, mkomar, pento.
Merges [42119] and [42120] to the 4.5 branch.
Fixes #42431 and #42401 for 4.5.

#14 @dd32
2 years ago

In 42234:

WPDB: Check that AUTH_SALT is not empty, Fix a PHP notice when AUTH_SALT is undefined.

Props jsonfry, mkomar, pento.
Merges [42119] and [42120] to the 4.4 branch.
Fixes #42431 and #42401 for 4.4.

#15 @dd32
2 years ago

In 42235:

WPDB: Check that AUTH_SALT is not empty, Fix a PHP notice when AUTH_SALT is undefined.

Props jsonfry, mkomar, pento.
Merges [42119] and [42120] to the 4.3 branch.
Fixes #42431 and #42401 for 4.3.

#16 @dd32
2 years ago

In 42236:

WPDB: Check that AUTH_SALT is not empty, Fix a PHP notice when AUTH_SALT is undefined.

Props jsonfry, mkomar, pento.
Merges [42119] and [42120] to the 4.2 branch.
Fixes #42431 and #42401 for 4.2.

#17 @dd32
2 years ago

In 42237:

WPDB: Check that AUTH_SALT is not empty, Fix a PHP notice when AUTH_SALT is undefined.

Props jsonfry, mkomar, pento.
Merges [42119] and [42120] to the 4.1 branch.
Fixes #42431 and #42401 for 4.1.

#18 @dd32
2 years ago

In 42238:

WPDB: Check that AUTH_SALT is not empty, Fix a PHP notice when AUTH_SALT is undefined.

Props jsonfry, mkomar, pento.
Merges [42119] and [42120] to the 4.0 branch.
Fixes #42431 and #42401 for 4.0.

#19 @dd32
2 years ago

In 42239:

WPDB: Check that AUTH_SALT is not empty, Fix a PHP notice when AUTH_SALT is undefined.

Props jsonfry, mkomar, pento.
Merges [42119] and [42120] to the 3.9 branch.
Fixes #42431 and #42401 for 3.9.

#20 @dd32
2 years ago

In 42240:

WPDB: Check that AUTH_SALT is not empty, Fix a PHP notice when AUTH_SALT is undefined.

Props jsonfry, mkomar, pento.
Merges [42119] and [42120] to the 3.8 branch.
Fixes #42431 and #42401 for 3.8.

#21 @dd32
2 years ago

In 42241:

WPDB: Check that AUTH_SALT is not empty, Fix a PHP notice when AUTH_SALT is undefined.

Props jsonfry, mkomar, pento.
Merges [42119] and [42120] to the 3.7 branch.
Fixes #42431 and #42401 for 3.7.

Note: See TracTickets for help on using tickets.