[GH-ISSUE #250] Security update: CVE-2017-5870 and CVE-2017-6086

kerem commented

2026-02-26 09:36:40 +03:00

Owner

Originally created by @Fmstrat on GitHub (Aug 8, 2018).
Original GitHub issue: https://github.com/opensolutions/ViMbAdmin/issues/250

It seems there are multiple cross-site request forgery (CSRF) vulnerabilities within the latest version of ViMbAdmin. Can we get a status on these fixes?

Seems the timeline states that you don't have time to fix bask in 2017, but wondering if any solution is in place now?

22/01/2017 : Initial discovery.
16/02/2017 : First contact with opensolutions.io
16/02/2017 : Advisory sent.
24/02/2017 : Reply from the owner, acknowledging the report and planning to fix the vulnerabilities.
13/03/2017 : Sysdream Labs request for an update.
29/03/2017 : Second request for an update.
29/03/2017 : Reply from the owner stating that he has no time to fix the issues.
03/05/2017 : Full disclosure.

References:

Originally created by @Fmstrat on GitHub (Aug 8, 2018). Original GitHub issue: https://github.com/opensolutions/ViMbAdmin/issues/250 It seems there are multiple cross-site request forgery (CSRF) vulnerabilities within the latest version of ViMbAdmin. Can we get a status on these fixes? Seems the timeline states that you don't have time to fix bask in 2017, but wondering if any solution is in place now? * 22/01/2017 : Initial discovery. * 16/02/2017 : First contact with opensolutions.io * 16/02/2017 : Advisory sent. * 24/02/2017 : Reply from the owner, acknowledging the report and planning to fix the vulnerabilities. * 13/03/2017 : Sysdream Labs request for an update. * 29/03/2017 : Second request for an update. * 29/03/2017 : Reply from the owner stating that he has no time to fix the issues. * 03/05/2017 : Full disclosure. References: - https://nvd.nist.gov/vuln/detail/CVE-2017-5870 - https://nvd.nist.gov/vuln/detail/CVE-2017-6086

kerem commented

2026-02-26 09:36:41 +03:00

Author

Owner

@barryo commented on GitHub (Aug 13, 2018):

No solution yet in place and no immediate free time to look at this. However this issue has bumped it back into my list of priorities.

If anyone wants to contribute to pushing this further up the list, the commercial support route would be a practical way of doing this.

ViMbAdmin is used by the vast majority of users as an in-house single-organisation tool and as such the exposure of both if these CVE's is - to my mind - much less than the CVE assigned scores.

@barryo commented on GitHub (Aug 13, 2018): No solution yet in place and no immediate free time to look at this. However this issue has bumped it back into my list of priorities. If anyone wants to contribute to pushing this further up the list, the [commercial support](https://www.vimbadmin.net/support.php) route would be a practical way of doing this. ViMbAdmin is used by the vast majority of users as an in-house single-organisation tool and as such the exposure of both if these CVE's is - to my mind - much less than the CVE assigned scores.

kerem commented

2026-02-26 09:36:41 +03:00

Author

Owner

@Fmstrat commented on GitHub (Aug 20, 2018):

@barryo As both of these are CSFR issues, I would certainly say they warrant the level of CVE assignment, even if they are in-house single-organization installs. These smaller systems mean smaller teams, and impact could be catastrophic for a small organization (or person) that can't figure out what's going on. That being said, CSFRs are easy to fix.

In any PHP header file that has a form in it, just add:

session_start();
if (empty($_SESSION['token'])) {
    if (function_exists('mcrypt_create_iv')) {
        $_SESSION['token'] = bin2hex(mcrypt_create_iv(32, MCRYPT_DEV_URANDOM));
    } else {
        $_SESSION['token'] = bin2hex(openssl_random_pseudo_bytes(32));
    }
}
$token = $_SESSION['token'];

In the <form> itself, add:

<input type="hidden" name="token" value="<?php $_SESSION['token']; ?>">

And after submit, before any functions are called, add:
edit Oops, cut and paste error below, fixed!

if (empty($_POST['token'] || (!empty($_POST['token'] && !hash_equals($_SESSION['token'], $_POST['token']))) {
         // Place code here to redirect to the login page.
}

This would eliminate both CVEs.

@Fmstrat commented on GitHub (Aug 20, 2018): @barryo As both of these are CSFR issues, I would certainly say they warrant the level of CVE assignment, even if they are in-house single-organization installs. These smaller systems mean smaller teams, and impact could be catastrophic for a small organization (or person) that can't figure out what's going on. That being said, CSFRs are easy to fix. In any PHP header file that has a form in it, just add: ``` session_start(); if (empty($_SESSION['token'])) { if (function_exists('mcrypt_create_iv')) { $_SESSION['token'] = bin2hex(mcrypt_create_iv(32, MCRYPT_DEV_URANDOM)); } else { $_SESSION['token'] = bin2hex(openssl_random_pseudo_bytes(32)); } } $token = $_SESSION['token']; ``` In the `<form>` itself, add: ``` <input type="hidden" name="token" value="<?php $_SESSION['token']; ?>"> ``` And after submit, before any functions are called, add: **edit** Oops, cut and paste error below, fixed! ``` if (empty($_POST['token'] || (!empty($_POST['token'] && !hash_equals($_SESSION['token'], $_POST['token']))) { // Place code here to redirect to the login page. } ``` This would eliminate both CVEs.

kerem commented

2026-02-26 09:36:41 +03:00

Author

Owner

@Fmstrat commented on GitHub (Aug 20, 2018):

I should note, the above solution uses hash_equals for checking the token, which is in PHP 5.6+ only, but available to earlier versions with the hash-compat library: https://github.com/indigophp/hash-compat

Some saying just using == or === is OK, but hash_equals is the true "secure" way to do it.

@Fmstrat commented on GitHub (Aug 20, 2018): I should note, the above solution uses `hash_equals` for checking the token, which is in PHP 5.6+ only, but available to earlier versions with the hash-compat library: https://github.com/indigophp/hash-compat Some saying just using `==` or `===` is OK, but `hash_equals` is the true "secure" way to do it.

kerem commented

2026-02-26 09:36:41 +03:00

Author

Owner

@Fmstrat commented on GitHub (Aug 21, 2018):

I'm about to fork and work on a PR.

@Fmstrat commented on GitHub (Aug 21, 2018): I'm about to fork and work on a PR.

kerem commented

2026-02-26 09:36:41 +03:00

Author

Owner

@Fmstrat commented on GitHub (Aug 22, 2018):

OK, I've started CSFR fixes on POSTS, but I realize the CVE also has GET issues, too. I wasn't sure of the details on your Zend implementation since much of it looks custom in your framework, so I did it the old fashion way, but it's clean, matches your code structure, and works.

Here's a commit with one form fixed. Let me know if you have issues with this structure as I'm going to use it across all forms, and then address the GET issues.

github.com/Fmstrat/ViMbAdmin@e82386f263

EDIT
This is for reference. I'll likely make createToken include all of the code to centralize, and centralize the token check. It's just to make sure you don't have any reason to not implement this way.

EDIT 2
Consolidated, see full diff: https://github.com/opensolutions/ViMbAdmin/compare/master...Fmstrat:master

EDIT 3
Since I'm not familiar with the full structure of things, I could keep moving with things, but let me know if you look at this and are like "Oh I could do this in an hour" because it would likely take me much longer.

@Fmstrat commented on GitHub (Aug 22, 2018): OK, I've started CSFR fixes on POSTS, but I realize the CVE also has GET issues, too. I wasn't sure of the details on your Zend implementation since much of it looks custom in your framework, so I did it the old fashion way, but it's clean, matches your code structure, and works. Here's a commit with one form fixed. Let me know if you have issues with this structure as I'm going to use it across all forms, and then address the GET issues. https://github.com/Fmstrat/ViMbAdmin/commit/e82386f2635692ea20042c3f117ee48529468c20 **EDIT** This is for reference. I'll likely make `createToken` include all of the code to centralize, and centralize the token check. It's just to make sure you don't have any reason to not implement this way. **EDIT 2** Consolidated, see full diff: https://github.com/opensolutions/ViMbAdmin/compare/master...Fmstrat:master **EDIT 3** Since I'm not familiar with the full structure of things, I could keep moving with things, but let me know if you look at this and are like *"Oh I could do this in an hour"* because it would likely take me much longer.

kerem commented

2026-02-26 09:36:41 +03:00

Author

Owner

@barryo commented on GitHub (Aug 22, 2018):

Hi @Fmstrat - thanks for the effort here.

Unfortunately, the way you've done it is not how we'd go about it (but it's close). We've already solved this in another project using the same backend code. Here are some links:

I've no issue looking at this when I have time, but if you want to look at it, the above should help.

To accept a more significant piece of code like this into the project, I'd also need you to sign an as yet unwritten CLA but something like this: https://github.com/inex/IXP-Manager/wiki/Contributor-License-Agreement

(side note: mcrypt is deprecated, please use random_bytes())

@barryo commented on GitHub (Aug 22, 2018): Hi @Fmstrat - thanks for the effort here. Unfortunately, the way you've done it is not how we'd go about it (but it's close). We've already solved this in another project using the same backend code. Here are some links: * https://github.com/inex/IXP-Manager/commit/47e2d7de1c90befdb0bdb799c83ad3667aa39108 * https://github.com/inex/IXP-Manager/commit/5a3ccfe62f58d9959ecfa2d68613a7fbc3234d54 * https://github.com/inex/IXP-Manager/commit/bc824f8abeaa167459bca7cb85bad56dbc7e3993 * https://github.com/inex/IXP-Manager/commit/75d82db682f77a6f9a48c6e0a2e4f48f8fbb0515 * https://github.com/inex/IXP-Manager/commit/2058294b3bcd25eb35cbceda5bf9ff9d2f88cf94#diff-49a23926d7ee71088d195d0367393668 I've no issue looking at this when I have time, but if you want to look at it, the above should help. To accept a more significant piece of code like this into the project, I'd also need you to sign an as yet unwritten CLA but something like this: https://github.com/inex/IXP-Manager/wiki/Contributor-License-Agreement *(side note: mcrypt is deprecated, please use random_bytes())*

kerem commented

2026-02-26 09:36:42 +03:00

Author

Owner

@Fmstrat commented on GitHub (Aug 22, 2018):

@barryo Yeaaaaa since I'm not that familiar with Zend, sounds like it would take me waaaaayyyy longer than makes sense given you already have a solution in place elsewhere. I'd love to help, but I might not be the right person (given the time I have currently).

@Fmstrat commented on GitHub (Aug 22, 2018): @barryo Yeaaaaa since I'm not that familiar with Zend, sounds like it would take me waaaaayyyy longer than makes sense given you already have a solution in place elsewhere. I'd love to help, but I might not be the right person (given the time I have currently).

Rows
Columns

[GH-ISSUE #250] Security update: CVE-2017-5870 and CVE-2017-6086 #200