Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: No validation rules for the placeholder #8638

Closed
Ukuser36 opened this issue Mar 19, 2024 · 3 comments
Closed

Bug: No validation rules for the placeholder #8638

Ukuser36 opened this issue Mar 19, 2024 · 3 comments
Labels
duplicate Issue or pull request duplicates an already existing issue/pull request wontfix Current code behavior being reported or fixed is intentional and won't be changed

Comments

@Ukuser36
Copy link

PHP Version

8.2

CodeIgniter4 Version

4.4.4

CodeIgniter4 Installation Method

Manual (zip or tar.gz)

Which operating systems have you tested for this bug?

Linux

Which server did you use?

apache

Database

MySql 8

What happened?

When providing an array to the Validation run() function, if a field isn't present and you also have a placeholder field in the same rule set (having been set with setRules() (such as required|is_unique[users.Email,UserID,{UserID}]) then $this->retrievePlaceholders($row, $data); is incorrectly returning results for fields that aren't in its scope of validation. As such the error gets trigggered No validation rules for the placeholder when the field isn't actually present in the ruleset.

Steps to Reproduce

For example, the following is passed to setRules()

<pre>Array
(
    [CompanyID] => Array
	(
	    [rules] => required
	)

    [FirstName] => Array
	(
	    [rules] => required
	)

    [LastName] => Array
	(
	    [rules] => required
	)

    [Email] => Array
	(
	    [rules] => required|is_unique[users.Email,UserID,{UserID}]
	)

)
</pre>

But the data provided to run() is:

Array
(
    [UserID] => 
    [UserType] => 2
    [CompanyID] => 1
    [FirstName] => New
    [LastName] => user
    [Email] => [email protected]
    [UserLevel] => user
    [ResetPass] => 0
)

The error is generated "No validation rules for the placeholder: UserID" but clearly this isn't in the rule set

Expected Output

No error regarding placeholders to be reported.

Anything else?

The solution is to clearly remove the data prior to submitting it to run() however run should only be validating data it needs to and not every field given.

@Ukuser36 Ukuser36 added the bug Verified issues on the current code behavior or pull requests that will fix them label Mar 19, 2024
@kenjis kenjis removed the bug Verified issues on the current code behavior or pull requests that will fix them label Mar 19, 2024
@kenjis
Copy link
Member

kenjis commented Mar 19, 2024

That is a bug in your app, not in CI4.

The error message may be not good, but what the message is trying to tell you is that you must validate the placeholder field. How do you think we can improve the error messages?

however run should only be validating data it needs to and not every field given.

No. You must validate all user input. Because attackers can send any data to your server.
If you don't validate all user input, it is just a bad practice.

See https://codeigniter4.github.io/userguide/libraries/validation.html#validation-placeholders

The rule is_unique[users.email,id,{id}] will be is_unique[users.email,id,4] in the example.
And it would generate a SQL statement with something like id != 4.
Then, what if an attacker send $_POST['id'] = '1; TRUNCATE TABLE users; --'?

In that case, CI4's Query Builder will protect it, so no SQL statement will be created that would cause TRUNCATE TABLE users; to be executed.

However, there is no need to process invalid data such as '1; TRUNCATE TABLE users; --' in the first place.
Your app should validate it before processing, and stop to process.

@kenjis kenjis closed this as completed Mar 19, 2024
@kenjis kenjis added the wontfix Current code behavior being reported or fixed is intentional and won't be changed label Mar 19, 2024
@kenjis
Copy link
Member

kenjis commented Mar 20, 2024

Duplicate of #7605

@kenjis kenjis marked this as a duplicate of #7605 Mar 20, 2024
@kenjis kenjis added the duplicate Issue or pull request duplicates an already existing issue/pull request label Mar 20, 2024
@kenjis
Copy link
Member

kenjis commented Mar 20, 2024

I sent a PR to improve the error message: #8639

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate Issue or pull request duplicates an already existing issue/pull request wontfix Current code behavior being reported or fixed is intentional and won't be changed
Projects
None yet
Development

No branches or pull requests

2 participants