Make WordPress Core

Opened 12 months ago

Closed 12 months ago

Last modified 12 months ago

#58181 closed defect (bug) (fixed)

wp_create_nonce() should not pass $action to wp_get_session_token()

Reported by: dd32's profile dd32 Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.3 Priority: normal
Severity: trivial Version:
Component: General Keywords: good-first-bug has-patch
Focuses: Cc:

Description

In [54218] / #35188 $action was accidentally passed to wp_get_session_token(). This function doesn't accept any parameters.

https://core.trac.wordpress.org/browser/trunk/src/wp-includes/pluggable.php?marks=2362#L2343

This has no current security implications or any effect at all, as wp_get_session_token() currently takes zero parameters.

Attachments (1)

58181.patch (557 bytes) - added by malavvasita 12 months ago.
Here is the patch

Download all attachments as: .zip

Change History (11)

#1 @SergeyBiryukov
12 months ago

  • Keywords needs-patch good-first-bug added

@malavvasita
12 months ago

Here is the patch

#2 @hztyfoon
12 months ago

Good Catch, Thanks for reporting it.

Thanks @malavvasita for the patch. I've checked your 58181.patch and looks like the php comment This function does not catch any $action after the wp_get_session_token function call is unnecessary.

I'm creating a PR so that we can move forward from there.
PR incoming.

#3 @malavvasita
12 months ago

@hztyfoon

May I take a chance to create a PR?
This would be my first contribution to WordPress.org.

#4 @hztyfoon
12 months ago

Sure. Welcome to WordPress trac in that case 👋.
Go Ahead please @malavvasita. 😊

Please make sure to add the link to this ticket https://core.trac.wordpress.org/ticket/58181 in the description of your PR.

Also, U can read this doc for more info: https://make.wordpress.org/core/handbook/contribute/git/github-pull-requests-for-code-review/

Last edited 12 months ago by hztyfoon (previous) (diff)

#5 @malavvasita
12 months ago

Thank you @hztyfoon for the input.

I've created a PR here: https://github.com/WordPress/wordpress-develop/pull/4374

#6 @hztyfoon
12 months ago

Thanks @malavvasita for the PR.
But looks like your PR has been linked with the wrong ticket (#35188).

Can U edit the description of your PR and move the following message at the very top of your message (the message U placed at the bottom of the description) or just simply remove everything and paste it in the description:

Trac ticket: https://core.trac.wordpress.org/ticket/58181

I believe if U put this 👆 at the very top of the description rather than putting it at the bottom, it will properly link the PR to this ticket.

This ticket was mentioned in PR #4374 on WordPress/wordpress-develop by malavvasita.


12 months ago
#7

  • Keywords has-patch added; needs-patch removed

Trac ticket: https://core.trac.wordpress.org/ticket/58181

Description:
In [54218] / #35188 $action was accidentally passed to wp_get_session_token(). This function doesn't accept any parameters.
In this PR, I have removed it to avoid consequences.

#8 @SergeyBiryukov
12 months ago

Thanks everyone! The PR looks good, I have edited the description to link to the correct ticket.

For future reference, PRs are linked to the first mentioned ticket URL. If there are multiple tickets, putting the current ticket's URL at the very top of the description as suggested in comment:6 should work, otherwise it can be linked to another ticket, which might not be the intended result.

Last edited 12 months ago by SergeyBiryukov (previous) (diff)

#9 @SergeyBiryukov
12 months ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 55685:

General: Don't pass $action to wp_get_session_token() in wp_create_nonce().

The parameter appears to have been passed by accident, as the function does not accept any parameters.

Follow-up to [54218].

Props malavvasita, hztyfoon, dd32.
Fixes #58181.

@SergeyBiryukov commented on PR #4374:


12 months ago
#10

Thanks for the PR! Merged in r55685.

Note: See TracTickets for help on using tickets.