Skip to content

login/login: guard post fields in validate() - #502

Merged
jsuto merged 1 commit into
jsuto:masterfrom
krzsztf1:fix-login-validate-undefined-key
Oct 7, 2026
Merged

jsuto merged 1 commit into
jsuto:masterfrom
krzsztf1:fix-login-validate-undefined-key

Conversation

@krzsztf1

Copy link
Copy Markdown
Contributor

ControllerLoginLogin::validate() reads two POST fields without checking
whether the keys exist:

private function validate() {
   if(strlen($this->request->post['username']) < 2){
      ...
   if($image->check($this->request->post['captcha']) != true) {

Any POST request dispatched to this controller without the login form produces
on PHP 8:

PHP Warning: Undefined array key "username" in
/var/piler/www/controller/login/login.php on line 131

strlen(null) is also deprecated since PHP 8.1, so line 131 warns twice on
newer versions.

Treating a missing field as empty keeps the existing behaviour - validation
fails with text_invalid_username either way.

The reads at lines 48, 51 and 95 are left alone: they only run after
validate() has returned true, so the key is guaranteed to exist.

Verified on a live 1.4.9 install: the warning is gone from the PHP-FPM log for
a POST without form fields, and normal login as well as the empty-username
validation error behave as before.

Environment: piler 1.4.9, PHP 8.5, Ubuntu 26.04, nginx.

validate() reads $this->request->post['username'] and ['captcha'] without
checking whether the keys exist. A POST request dispatched to this controller
without the login form produces on PHP 8:

  Warning: Undefined array key "username"

strlen(null) is also deprecated since PHP 8.1, so line 131 warns twice on
newer versions.

Treating a missing field as empty keeps the existing behaviour - validation
fails either way. The reads at lines 48, 51 and 95 are left alone: they run
only after validate() has returned true, so the key is guaranteed to exist.
@krzsztf1
krzsztf1 requested a review from jsuto as a code owner September 11, 2026 11:31

@jsuto jsuto left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@jsuto
jsuto merged commit ea9619d into jsuto:master Oct 7, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants