Skip to content

Commit 6eaa56f

Browse files
boonebgorgesboonebgorges
committed
Don't improperly cast IDs when fetching post, user, or term objects.
Blindly casting passed IDs to integers can generate false positives when the ID is cast to `1`. Props deeptiboddapati. Fixes #37738. git-svn-id: https://develop.svn.wordpress.org/trunk@38381 602fd350-edb4-49c9-b593-d223f7449a82
1 parent 8b431f6 commit 6eaa56f

6 files changed

Lines changed: 325 additions & 6 deletions

File tree

‎src/wp-includes/class-wp-comment.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,11 +191,12 @@ final class WP_Comment {
191191
public static function get_instance( $id ) {
192192
global $wpdb;
193193

194-
$comment_id = (int) $id;
195-
if ( ! $comment_id ) {
194+
if ( ! is_numeric( $id ) || $id != floor( $id ) || ! $id ) {
196195
return false;
197196
}
198197

198+
$comment_id = (int) $id;
199+
199200
$_comment = wp_cache_get( $comment_id, 'comment' );
200201

201202
if ( ! $_comment ) {

‎src/wp-includes/class-wp-post.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -210,9 +210,11 @@ final class WP_Post {
210210
public static function get_instance( $post_id ) {
211211
global $wpdb;
212212

213-
$post_id = (int) $post_id;
214-
if ( ! $post_id )
213+
if ( ! is_numeric( $post_id ) || $post_id != floor( $post_id ) || ! $post_id ) {
215214
return false;
215+
}
216+
217+
$post_id = (int) $post_id;
216218

217219
$_post = wp_cache_get( $post_id, 'posts' );
218220

‎src/wp-includes/class-wp-term.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,11 +125,12 @@ final class WP_Term {
125125
public static function get_instance( $term_id, $taxonomy = null ) {
126126
global $wpdb;
127127

128-
$term_id = (int) $term_id;
129-
if ( ! $term_id ) {
128+
if ( ! is_numeric( $term_id ) || $term_id != floor( $term_id ) || ! $term_id ) {
130129
return false;
131130
}
132131

132+
$term_id = (int) $term_id;
133+
133134
$_term = wp_cache_get( $term_id, 'terms' );
134135

135136
// If there isn't a cached version, hit the database.

‎tests/phpunit/tests/comment/wpComment.php‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
<?php
2+
3+
/**
4+
* @group comment
5+
*/
6+
class Tests_Term_WpComment extends WP_UnitTestCase {
7+
protected static $comment_id;
8+
9+
public static function wpSetUpBeforeClass( $factory ) {
10+
global $wpdb;
11+
12+
// Ensure that there is a comment with ID 1.
13+
$comment_1 = WP_Comment::get_instance( 1 );
14+
if ( ! $comment_1 ) {
15+
$wpdb->insert( $wpdb->comments, array(
16+
'comment_ID' => 1,
17+
) );
18+
19+
clean_comment_cache( 1 );
20+
}
21+
22+
self::$comment_id = self::factory()->comment->create();
23+
}
24+
25+
public static function wpTearDownAfterClass() {
26+
wp_delete_comment( 1, true );
27+
wp_delete_comment( self::$comment_id, true );
28+
}
29+
30+
/**
31+
* @ticket 37738
32+
*/
33+
public function test_get_instance_should_work_for_numeric_string() {
34+
$found = WP_Comment::get_instance( (string) self::$comment_id );
35+
36+
$this->assertEquals( self::$comment_id, $found->comment_ID );
37+
}
38+
39+
/**
40+
* @ticket 37738
41+
*/
42+
public function test_get_instance_should_fail_for_negative_number() {
43+
$found = WP_Comment::get_instance( -self::$comment_id );
44+
45+
$this->assertFalse( $found );
46+
}
47+
48+
/**
49+
* @ticket 37738
50+
*/
51+
public function test_get_instance_should_fail_for_non_numeric_string() {
52+
$found = WP_Comment::get_instance( 'abc' );
53+
54+
$this->assertFalse( $found );
55+
}
56+
57+
/**
58+
* @ticket 37738
59+
*/
60+
public function test_get_instance_should_fail_for_bool() {
61+
$found = WP_Comment::get_instance( true );
62+
63+
$this->assertFalse( $found );
64+
}
65+
66+
/**
67+
* @ticket 37738
68+
*/
69+
public function test_get_instance_should_succeed_for_float_that_is_equal_to_post_id() {
70+
$found = WP_Comment::get_instance( 1.0 );
71+
72+
$this->assertEquals( 1, $found->comment_ID );
73+
}
74+
75+
/**
76+
* @ticket 37738
77+
*/
78+
public function test_get_instance_should_fail_for_float() {
79+
$found = WP_Comment::get_instance( 1.6 );
80+
81+
$this->assertFalse( $found );
82+
}
83+
84+
/**
85+
* @ticket 37738
86+
*/
87+
public function test_get_instance_should_fail_for_array() {
88+
$found = WP_Comment::get_instance( array( 1 ) );
89+
90+
$this->assertFalse( $found );
91+
}
92+
93+
/**
94+
* @ticket 37738
95+
*/
96+
public function test_get_instance_should_fail_for_class() {
97+
$class = new stdClass();
98+
$found = WP_Comment::get_instance( $class );
99+
100+
$this->assertFalse( $found );
101+
}
102+
}

‎tests/phpunit/tests/post/wpPost.php‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
<?php
2+
3+
/**
4+
* @group post
5+
*/
6+
class Tests_Post_WpPost extends WP_UnitTestCase {
7+
protected static $post_id;
8+
9+
public static function wpSetUpBeforeClass( $factory ) {
10+
global $wpdb;
11+
12+
// Ensure that there is a post with ID 1.
13+
if ( ! get_post( 1 ) ) {
14+
$wpdb->insert( $wpdb->posts, array(
15+
'ID' => 1,
16+
'post_title' => 'Post 1',
17+
) );
18+
}
19+
20+
self::$post_id = self::factory()->post->create();
21+
}
22+
23+
public static function wpTearDownAfterClass() {
24+
wp_delete_post( 1, true );
25+
wp_delete_post( self::$post_id, true );
26+
}
27+
28+
/**
29+
* @ticket 37738
30+
*/
31+
public function test_get_instance_should_work_for_numeric_string() {
32+
$found = WP_Post::get_instance( (string) self::$post_id );
33+
34+
$this->assertSame( self::$post_id, $found->ID );
35+
}
36+
37+
/**
38+
* @ticket 37738
39+
*/
40+
public function test_get_instance_should_fail_for_negative_number() {
41+
$found = WP_Post::get_instance( -self::$post_id );
42+
43+
$this->assertFalse( $found );
44+
}
45+
46+
/**
47+
* @ticket 37738
48+
*/
49+
public function test_get_instance_should_fail_for_non_numeric_string() {
50+
$found = WP_Post::get_instance( 'abc' );
51+
52+
$this->assertFalse( $found );
53+
}
54+
55+
/**
56+
* @ticket 37738
57+
*/
58+
public function test_get_instance_should_fail_for_bool() {
59+
$found = WP_Post::get_instance( true );
60+
61+
$this->assertFalse( $found );
62+
}
63+
64+
/**
65+
* @ticket 37738
66+
*/
67+
public function test_get_instance_should_succeed_for_float_that_is_equal_to_post_id() {
68+
$found = WP_Post::get_instance( 1.0 );
69+
70+
$this->assertSame( 1, $found->ID );
71+
}
72+
73+
/**
74+
* @ticket 37738
75+
*/
76+
public function test_get_instance_should_fail_for_float() {
77+
$found = WP_Post::get_instance( 1.6 );
78+
79+
$this->assertFalse( $found );
80+
}
81+
82+
/**
83+
* @ticket 37738
84+
*/
85+
public function test_get_instance_should_fail_for_array() {
86+
$found = WP_Post::get_instance( array( 1 ) );
87+
88+
$this->assertFalse( $found );
89+
}
90+
91+
/**
92+
* @ticket 37738
93+
*/
94+
public function test_get_instance_should_fail_for_class() {
95+
$class = new stdClass();
96+
$found = WP_Post::get_instance( $class );
97+
98+
$this->assertFalse( $found );
99+
}
100+
}

‎tests/phpunit/tests/term/wpTerm.php‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
<?php
2+
3+
/**
4+
* @group taxonomy
5+
*/
6+
class Tests_Term_WpTerm extends WP_UnitTestCase {
7+
protected static $term_id;
8+
9+
public function setUp() {
10+
parent::setUp();
11+
register_taxonomy( 'wptests_tax', 'post' );
12+
}
13+
14+
public static function wpSetUpBeforeClass( $factory ) {
15+
global $wpdb;
16+
17+
register_taxonomy( 'wptests_tax', 'post' );
18+
19+
// Ensure that there is a term with ID 1.
20+
if ( ! get_term( 1 ) ) {
21+
$wpdb->insert( $wpdb->terms, array(
22+
'term_id' => 1,
23+
) );
24+
25+
$wpdb->insert( $wpdb->term_taxonomy, array(
26+
'term_id' => 1,
27+
'taxonomy' => 'wptests_tax',
28+
) );
29+
30+
clean_term_cache( 1, 'wptests_tax' );
31+
}
32+
33+
self::$term_id = self::factory()->term->create( array( 'taxonomy' => 'wptests_tax' ) );
34+
}
35+
36+
public static function wpTearDownAfterClass() {
37+
wp_delete_term( 1, 'wptests_tax' );
38+
wp_delete_term( self::$term_id, 'wptests_tax' );
39+
}
40+
41+
/**
42+
* @ticket 37738
43+
*/
44+
public function test_get_instance_should_work_for_numeric_string() {
45+
$found = WP_Term::get_instance( (string) self::$term_id );
46+
47+
$this->assertSame( self::$term_id, $found->term_id );
48+
}
49+
50+
/**
51+
* @ticket 37738
52+
*/
53+
public function test_get_instance_should_fail_for_negative_number() {
54+
$found = WP_Term::get_instance( -self::$term_id );
55+
56+
$this->assertFalse( $found );
57+
}
58+
59+
/**
60+
* @ticket 37738
61+
*/
62+
public function test_get_instance_should_fail_for_non_numeric_string() {
63+
$found = WP_Term::get_instance( 'abc' );
64+
65+
$this->assertFalse( $found );
66+
}
67+
68+
/**
69+
* @ticket 37738
70+
*/
71+
public function test_get_instance_should_fail_for_bool() {
72+
$found = WP_Term::get_instance( true );
73+
74+
$this->assertFalse( $found );
75+
}
76+
77+
/**
78+
* @ticket 37738
79+
*/
80+
public function test_get_instance_should_succeed_for_float_that_is_equal_to_post_id() {
81+
$found = WP_Term::get_instance( 1.0 );
82+
83+
$this->assertSame( 1, $found->term_id );
84+
}
85+
86+
/**
87+
* @ticket 37738
88+
*/
89+
public function test_get_instance_should_fail_for_float() {
90+
$found = WP_Term::get_instance( 1.6 );
91+
92+
$this->assertFalse( $found );
93+
}
94+
95+
/**
96+
* @ticket 37738
97+
*/
98+
public function test_get_instance_should_fail_for_array() {
99+
$found = WP_Term::get_instance( array( 1 ) );
100+
101+
$this->assertFalse( $found );
102+
}
103+
104+
/**
105+
* @ticket 37738
106+
*/
107+
public function test_get_instance_should_fail_for_class() {
108+
$class = new stdClass();
109+
$found = WP_Term::get_instance( $class );
110+
111+
$this->assertFalse( $found );
112+
}
113+
}

0 commit comments

Comments
 (0)