laminas / laminas-captcha

Generate and validate CAPTCHAs using Figlets, images, ReCaptcha, and more
https://docs.laminas.dev/laminas-captcha/
BSD 3-Clause "New" or "Revised" License
25 stars 14 forks source link

setExpirationHops() bug #1

Open weierophinney opened 4 years ago

weierophinney commented 4 years ago

The bug can recur very easily.

<?php
use Zend\Captcha\Figlet;

require 'vendor/autoload.php';

$captcha = new Figlet();

if (empty($_GET['input'])) {
    $captcha->getSession()->setExpirationHops(1);
    echo $id = $captcha->generate() . '<br>';
    echo $word = $captcha->getWord();
} else {
    var_dump(
        $captcha->isValid(
            [
                'id' => $_GET['id'],
                'input' => $_GET['input']
            ]));
}

In generate stage I set expirationHops to 1. Validation stage can be executed many times and will always output bool(true).

I dive into the code and find the cause. In class Zend\Captcha\AbstractWord (line 253)

    /**
     * Get session object
     *
     * @throws Exception\InvalidArgumentException
     * @return Container
     */
    public function getSession()
    {
        if (! isset($this->session) || (null === $this->session)) {
            $id = $this->getId();
            if (! class_exists($this->sessionClass)) {
                throw new Exception\InvalidArgumentException("Session class $this->sessionClass not found");
            }
            $this->session = new $this->sessionClass('Zend_Form_Captcha_' . $id);
            $this->session->setExpirationHops(1, null);
            $this->session->setExpirationSeconds($this->getTimeout());
        }
        return $this->session;
    }

In validation stage "$this->session->setExpirationHops(1, null);" will reset expirationHops. But expirationHops will not take effect in the same request. So after isValid() hops will always be 1.


Originally posted by @ares333 at https://github.com/zendframework/zend-captcha/issues/37

ellyxc commented 4 years ago

i patched the getSession function with

public function getSession()
    {
        if (! isset($this->session) || (null === $this->session)) {
            $id = $this->getId();
            if (! class_exists($this->sessionClass)) {
                throw new Exception\InvalidArgumentException("Session class $this->sessionClass not found");
            }
            $this->session = new $this->sessionClass('Laminas_Form_Captcha_' . $id);
            if ($this->session->initialised !== true) {
                $this->session->initialised = true;
                $this->session->setExpirationHops(1, null);
                $this->session->setExpirationSeconds($this->getTimeout());
            }
        }
        return $this->session;
    }

I am not sure, if this is a best practice, or the right place to fix this. but it is working on my case. i could create a pull request for this one.