View Issue Details

IDProjectCategoryView StatusLast Update
0017316phplist applicationMessage Send Processpublic24-11-14 22:52
Reporternymisoa 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.0.7 
Target Version3.0.8Fixed in Version3.0.8 
Summary0017316: Processing the Queue via PHP-CGI cURL Broken in 3.0.7
DescriptionI run php-cgi. I've processed the queue via an hourly cron job to the attached php file (not calling cURL on the cron tab command line) for years. Seems like the requirement to have a session token has broken the URL call to:

http://mydomain.com/phplist/admin/?page=pageaction&action=processqueue&login=USERID&password=PW

The cURL call returns "Error, incorrect session token".

Is there a documented method to process the queue via cURL since session tokens are now required? Losing this functionality is a step backwards in my opinion.
Additional InformationThe Manually Process Queue constant is defined as 1.
TagsNo tags attached.

Relationships

related to 0017358 resolvedmichiel phplist application allow remote cron calls with secret 

Activities

nymisoa

21-08-14 18:13

reporter  

cron.php (294 bytes)

michiel

21-08-14 18:44

manager   ~0054669


Ah, interesting one. Yes, that's a side effect of more security. I guess you can hack out the check for now, but it may be good to update it to allow the option to switch off the tokencheck in these situations.

nymisoa

21-08-14 19:08

reporter   ~0054670

I hacked out the call to verifyCsrfGetToken(); in ../admin/actions/processqueue.php and the cURL call worked properly.

I think the best way to switch off the tokencheck for this is to pass a $_GET variable in the cURL call. Maybe there's a way to set a custom header in the cURL call that phplist would recognize and bypass the tokencheck.

michiel

21-08-14 19:17

manager   ~0054671

well, that would make the tokenCheck useless, as the whole point is to verify access and avoid XSRF.

I think allowing an option to selectively switch it off might work, so that those who need it can do it, as long as they acknowledge and accept the consequences

duncanc

22-08-14 09:27

developer   ~0054676

I have installed phplist for people whose hosting accounts do not have access to php cli, only to php cgi. So they use a cron command similar to this, which is in essence the same as the curl command:

php /home/xxx/public_html/lists/admin/index.php page=pageaction action=processqueue login=admin password=yyyy

The extra XSRF processing also breaks these cron jobs.

If there is somewhere prominent to put it, I think that the release information for 3.0.7 needs a warning about it breaking cron jobs that use php cgi.

gingerling

22-08-14 11:57

manager   ~0054677

Duncan, thanks for mentioning this! Can you blog this up please on community.phplist.com and we will publish it. Let me know if you need help accessing the blog and using it.

Also, if you can make sure there is a solution explained in the blog, even if it's quite general, that would be cool :)

 Ax

duncanc

22-08-14 15:08

developer   ~0054678

Last edited: 22-08-14 15:35

View 2 revisions

I think that Michiel needs to decide how to handle this problem. If a change to phplist is needed then it will warrant a further release.

The immediate message is simple - If your cron job to process the queue uses php-cgi or curl then do not upgrade to 3.0.7 because the phplist code security changes stop that method from working.

nymisoa

23-08-14 23:00

reporter   ~0054686

I still think that a 12-16 random character key can be added in config.php and used as a $_GET variable so the cURL call can include it to bypass the XSRF processing. Only someone with access to the config.php file will know what the key is. Probably stronger security that what most users use as a password to access the admin pages.

michiel

24-08-14 09:44

manager   ~0054687

no, that's not a solution. The random code is different every time.

michiel

24-08-14 19:48

manager   ~0054688

ah, sorry, nymisoa, re-reading your comment, you are not referring to the existing XSRF tokens, but suggesting a new one as a new authentication system.

that may be a route to go down.

duncanc

25-08-14 13:36

developer   ~0054689

A different view on this. As the command is authenticating with login and password, is there a need to also validate for XSRF?

I'm not sure whether providing login and password as part of the URL is a deliberate feature, or whether it is a side-effect. If it is deliberate then maybe the XSRF need not be applied.

nymisoa

25-08-14 13:40

reporter   ~0054690

Last edited: 25-08-14 16:47

View 2 revisions

No problem. What I was thinking was something like this:

in config.php

define("secret_token","4pGrABLvLzYCPTZj");//some random, user-developed, tough to guess URL-happy string with default set to "0"

in line 1 of ../admin/actions/processqueue.php

if (!defined('secret_token') || $_GET['secret_token'] !== secret_token) verifyCsrfGetToken();

That way if someone wants to set the ability to process the queue via cURL AND they know the token they've configured. This also wouldn't affect the vast majority of users who process the queue from a browser environment.

Thanks for reading!

michiel

02-09-14 09:37

manager   ~0054808

Duncan, in reply to 0017316:0054689, when php-cli is used, the username and password parameters are not needed. For me the awkward thing is that I do not have a php-cgi system anywhere, so I have to second guess the situation, or rely on reports like this.

The thing is that presumably the php-cgi binary still treats inputs like "$_GET" etc (and not argv), which means they cannot be distinguished from a normal web-environment. Basically if PHP_SAPI == cgi, you don't know if it's commandline or Web-environment.

deajan

03-09-14 06:40

reporter   ~0054836

Hello,

Sorry to add by 2 cents but whenever you use php-fpm instead of mod_php on apache (which becomes quite usual these days), you'll end up using php-cgi in web environment anyway.

How about a predefined secret cron key like where the cronjob would look like

/path/to/index.php -pprocessqueue -c/path/to/config/config.php -ssecretkey

and which would allow a cronjob to bypass the session check ?

Regards,
Ozy.

duncanc

03-09-14 08:18

reporter   ~0054837

Michiel, re 0017316:0054808, you are answering a slightly different question to the one that I tried to ask. But the question of how to identify whether php is running from a command line/cron job or through a web server is something that I have briefly looked into. I will open a new issue for that.

Rephrasing my original question - because the curl command includes the admin password can the csrf token be taken for granted only for that single request?

http://mydomain.com/phplist/admin/?page=pageaction&action=processqueue&login=USERID&password=PW

If the login credentials are valid then set $_GET['tk'] to the session token so that further validation of $_GET['tk'] will be successful. Does this create any security hole? Add the line to the else part of this processing in admin/index.php

    $loginresult = $GLOBALS["admin_auth"]->validateLogin($_REQUEST["login"],$_REQUEST["password"]);
    if (!$loginresult[0]) {
      $_SESSION["adminloggedin"] = "";
      $_SESSION["logindetails"] = "";
      $page = "login";
      logEvent(sprintf($GLOBALS['I18N']->get('invalid login from %s, tried logging in as %s'),$_SERVER['REMOTE_ADDR'],$_REQUEST["login"]));
      $msg = $loginresult[1];
    } else {
      $_SESSION["adminloggedin"] = $_SERVER["REMOTE_ADDR"];
      $_SESSION["logindetails"] = array(
        "adminname" => $_REQUEST["login"],
        "id" => $loginresult[0],
        "superuser" => $admin_auth->isSuperUser($loginresult[0]),
        "passhash" => sha1($_REQUEST["password"]),
      );
      0016692 - make sure admin permissions apply at first login
      $GLOBALS["admin_auth"]->validateAccount($_SESSION["logindetails"]["id"]);
      if (!empty($_POST["page"])) {
        $page = preg_replace('/\W+/','',$_POST["page"]);
      }
      $_GET['tk'] = $_SESSION['csrf_token'];
    }


deajan, re 0017316:0054836, the cron command that you are using is for php-cli and the csrf validation is not used for that.

deajan

03-09-14 14:11

reporter   ~0054841

My bad :)

michiel

04-09-14 08:59

developer   ~0054848


The problem with having the login/password on the GET URL is that it increases the chance of it leaking. When it has leaked the attacker will have access to the entire system, which is not good.

I think I will resolve it as follows:

1. set a secret in the config that is used ONLY for the processqueue call.
2. allow calling the processqueue remotely, without login/password and with the secret.

That way, an attacker discovering the secret will only be able to run the queue, but will not have access to the rest of the system. If the queue running is invoked this way, we keep output to a minimum, to avoid leaking additional information to the attacker.

nymisoa

04-09-14 12:37

reporter   ~0054855

michiel...elegant and appropriate solution. I'd suggest a similar approach for processing bounces which right now isn't affected by the XSRF work you've done. I know I process bounces via cURL weekly using login and password in the URL.

Your solution also eliminates the best practice of setting up an admin with privileges restricted to processing the queue and bounces which is what I did for the cURL calls. If somehow someone got those credentials in the cURL call, all they could do is process the queue and bounces which is quite harmless.

Thank you for all you do for us phpList users!

michiel

04-09-14 15:24

developer   ~0054857

Yes, true this will apply to processbounces as well. I think the best thing to do is to set up a "cron" page, which allows a single cURL call and calls queue and bounce processing based on some settings.

That makes it easier to extend the functionality without the need to set up further crons.

That would justify calling it version 3.1.0 as that's new functionality.

michiel

05-09-14 15:36

developer   ~0054881

For a quick fix release, I've removed the XSRF check on the processqueue page. I will roll out version 3.0.8 by monday latest.

The issue to properly resolve this continues on 0017316

michiel

24-11-14 22:52

developer   ~0055731

the remote processing with a secret has been implemented in beta stage. See 0017358

Issue History

Date Modified Username Field Change
21-08-14 18:13 nymisoa New Issue
21-08-14 18:13 nymisoa File Added: cron.php
21-08-14 18:44 michiel Note Added: 0054669
21-08-14 18:44 michiel Target Version => 3.0.X
21-08-14 19:08 nymisoa Note Added: 0054670
21-08-14 19:17 michiel Note Added: 0054671
22-08-14 09:27 duncanc Note Added: 0054676
22-08-14 11:57 gingerling Note Added: 0054677
22-08-14 15:08 duncanc Note Added: 0054678
22-08-14 15:35 duncanc Note Edited: 0054678 View Revisions
23-08-14 23:00 nymisoa Note Added: 0054686
24-08-14 09:44 michiel Note Added: 0054687
24-08-14 19:48 michiel Note Added: 0054688
25-08-14 13:36 duncanc Note Added: 0054689
25-08-14 13:40 nymisoa Note Added: 0054690
25-08-14 16:47 nymisoa Note Edited: 0054690 View Revisions
02-09-14 09:37 michiel Note Added: 0054808
03-09-14 06:40 deajan Note Added: 0054836
03-09-14 08:18 duncanc Note Added: 0054837
03-09-14 14:11 deajan Note Added: 0054841
04-09-14 08:59 michiel Note Added: 0054848
04-09-14 08:59 michiel Target Version 3.0.X => 3.0.8
04-09-14 12:37 nymisoa Note Added: 0054855
04-09-14 15:24 michiel Note Added: 0054857
05-09-14 15:35 michiel Relationship added related to 0017358
05-09-14 15:36 michiel Note Added: 0054881
05-09-14 15:36 michiel Status new => resolved
05-09-14 15:36 michiel Fixed in Version => 3.0.8
05-09-14 15:36 michiel Resolution open => fixed
24-11-14 22:52 michiel Note Added: 0055731