Make WordPress Core

Opened 6 months ago

Last modified 7 weeks ago

#61732 assigned defect (bug)

get_calendar() will use invalid dates in SQL queries

Reported by: leedxw's profile leedxw Owned by: pbearne's profile pbearne
Milestone: 6.8 Priority: normal
Severity: normal Version:
Component: Date/Time Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

get_calendar() will use a client provided value for an SQL date request without checking validity.

Checking the logs recently, I can see errors along the lines of

PHP message: WordPress database error Incorrect DATETIME value: '2171-94-01' for query SELECT MONTH(post_date) AS month, YEAR(post_date) AS year ...

Where the value was not a valid since the month was not between 01 and 12 inclusive.

Investigating, what I was seeing was:

  • A bunch of junk requests that had random variables applied
    GET /some_nonexistant_random_page?m=la2m,1o7tfe1poe9h_4ksa8,w876
    
  • The theme uses something that uses widget, that then uses get_calendar()
  • WP_Query::parse_query() was helpfully stripping out non numbers into the global $m - but there are no validity checks on the result.
  • get_calendar() was using a substring of the junk $m as the base for month.
  • SQL complains about an invalid date

Attachments (1)

calendar_month.patch (928 bytes) - added by leedxw 6 months ago.
Patch to ensure month value is valid

Download all attachments as: .zip

Change History (8)

@leedxw
6 months ago

Patch to ensure month value is valid

#1 @sabernhardt
5 months ago

  • Component changed from General to Date/Time

Related: #41011

#2 @hellofromTonya
4 months ago

  • Keywords has-patch needs-unit-tests needs-testing-info needs-testing added
  • Version 6.6 deleted

Hello @leedxw,

Welcome to WordPress Core's Trac. Thank you for reporting this issue.

I'm doing triage and will update the ticket to help it be more actionable:

  • Version: this area of the code predates 6.6 and may go back 20+ years (i.e. r933). For now, removing the Version that introduced the issue. Once investigated, the Version can be updated.
  • Testing: Adding needs-testing, as bug reproduction test report(s) are helpful.
  • Patch: there is a patch - marking with has-patch.
  • Tests: will be great to also add automated PHPUnit tests.

@leedxw can you please share step-by-step instructions for how to reproduce the issue? Thank you.

#3 @ngoncalves
3 months ago

I have the same issue and this is the errors I can see in logs.

PHP message: WordPress database error Incorrect DATETIME value: '2024-11-31 23:59:59' for query SELECT MONTH(post_date) AS month, YEAR(post_date) AS year
          FROM wp_15_posts
          WHERE post_date > '2024-11-31 23:59:59'
          AND post_type = 'post' AND post_status = 'publish'
          ORDER BY post_date ASC
          LIMIT 1 made by require('wp-blog-header.php'), require_once('wp-includes/template-loader.php'), include('wp-includes/template-canvas.php'), get_the_block_template_html, do_blocks, render_block, WP_Block->render, render_block_core_template_part, do_blocks, render_block, WP_Block->render, WP_Block->render, WP_Block->render, WP_Block->render, render_block_core_calendar, get_calendar, QM_DB->query


In the get_calendar function in wp-includes/general-template.php, before it tries to figure out if it should show a link to the next month with posts, it creates a unix timestamp of the first of the month at midnight in order to get the last day of that month.
Depending on the timezone configured, this can make the date rollback to the last day of the previous month. This can have 2 consequences:

  • if the previous month has more days than the current month, it crashes with the above error.
  • if the previous month has less days than the current month, it won't crash but there will be weird due to not being properly clamped to the end of the month

Exemple:

 $unixmonth = mktime( 0, 0, 0, $thismonth, 1, $thisyear );
 $last_day  = gmdate( 't', $unixmonth );                   

Assuming $thismonth = 11 and $thisyear = 2024,
With Europe/Paris timezone: $unixmonth = 1730415600 $lastday = 31
With UTC timezone timezone: $unixmonth 1730419200 $lastday = 30

To fix that issue, creating $unixmonth with a day of 2 would stop any possibility of rollback.

This ticket was mentioned in PR #7864 on WordPress/wordpress-develop by @pbearne.


2 months ago
#4

  • Keywords has-unit-tests added; needs-unit-tests removed

Introduce comprehensive PHPUnit tests for the get_calendar() function to verify correct calendar HTML output for various conditions. Also, fix an issue in the function where invalid month values reset to January.

#5 @pbearne
2 months ago

  • Keywords needs-testing-info needs-testing removed
  • Milestone changed from Awaiting Review to Future Release
  • Owner set to pbearne
  • Status changed from new to assigned

#6 @pbearne
2 months ago

  • Milestone changed from Future Release to 6.8

#7 @ngoncalves
7 weeks ago

The current patch doesn't fix the issues with get_calendar when using timezones ahead of UTC that I wrote about above.

As I'm not familiar with the WordPress codebase, I'm unsure of how to fix it in a way that would be merged so I'll try to showcase the issues using tests: Here are 2 tests which fails when you set the timezone to anything ahead of UTC (like UTC+1), but passes if you remove the date_default_timezone_set statements.

<?php
        /*
         * Test that the get_calendar function return HTML that represents the last day of October, day 31
         * in positive non UTC timezone (i.e: UTC+1)
        */
        public function testGet_calendar_returns_post_day_31_in_october_in_non_UTC_timezone() {
                $day_31_in_calendar_html = '<td><a href="http://example.org/?m=20241031" aria-label="Posts published on October 31, 2024">31</a>';
                $post_id  = self::factory()->post->create( array( 'post_date' => '2024-10-31 10:00:00' ) );
                global $year, $monthnum;
                $year = 2024;
                $monthnum = 10;

                date_default_timezone_set('Europe/Paris');
                $output = get_calendar( true, false);
                $this->assertStringContainsString( $day_31_in_calendar_html, $output );
        }
       /*
         * Test that get_calendar function doesn't trigger wpdb errors in positive non UTC timezone (i.e: UTC+1)
         *
         */
        public function testGet_calendar_no_database_errors_in_non_UTC_timezone() {
                $post_id  = self::factory()->post->create( array( 'post_date' => '2024-11-15 10:00:00' ) );
                global $year, $monthnum, $wpdb;
                $year = 2024;
                $monthnum = 11;

                date_default_timezone_set('Europe/Paris');

                $output = get_calendar( true, false);
                $this->assertSame('', $wpdb->last_error);
                date_default_timezone_set('UTC');
        }
Note: See TracTickets for help on using tickets.