phplist

NOTE:: Before reporting an issue, make sure you are running the latest version, currently 3.3.1


View Issue Details Jump to Notes ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0017316phplist applicationMessage Send Processpublic21-08-14 19:1324-11-14 22:52
Reporternymisoa 
PrioritynormalSeverityminorReproducibilityalways
StatusresolvedResolutionfixed 
PlatformOSOS Version
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.
Attached Files? file icon cron.php [^] (294 bytes) 21-08-14 19:13

- Relationships Relation Graph ] Dependency Graph ]
related to 0017358resolvedmichiel phplist application allow remote cron calls with secret 

-  Notes
(0054669)
michiel (manager)
21-08-14 19:44


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.
(0054670)
nymisoa (reporter)
21-08-14 20:08

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.
(0054671)
michiel (manager)
21-08-14 20:17

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
(0054676)
duncanc (developer)
22-08-14 10:27

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.
(0054677)
gingerling (administrator)
22-08-14 12:57

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
(0054678)
duncanc (developer)
22-08-14 16:08
edited on: 22-08-14 16:35

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.

(0054686)
nymisoa (reporter)
24-08-14 00:00

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.
(0054687)
michiel (manager)
24-08-14 10:44

no, that's not a solution. The random code is different every time.
(0054688)
michiel (manager)
24-08-14 20:48

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.
(0054689)
duncanc (developer)
25-08-14 14:36

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.
(0054690)
nymisoa (reporter)
25-08-14 14:40
edited on: 25-08-14 17:47

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!

(0054808)
michiel (manager)
02-09-14 10:37

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.
(0054836)
deajan (reporter)
03-09-14 07:40

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.
(0054837)
duncanc (developer)
03-09-14 09:18

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.
(0054841)
deajan (reporter)
03-09-14 15:11

My bad :)
(0054848)
michiel (manager)
04-09-14 09:59


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.

(0054855)
nymisoa (reporter)
04-09-14 13:37

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!
(0054857)
michiel (manager)
04-09-14 16:24

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.
(0054881)
michiel (manager)
05-09-14 16:36

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
(0055731)
michiel (manager)
24-11-14 22:52

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


Copyright © 2000 - 2017 MantisBT Team
Powered by Mantis Bugtracker