Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mirasvit_Helpdesk vulnerable up to version 1.5.14 #40

Merged
merged 3 commits into from
Jul 25, 2019

Conversation

mzeis
Copy link
Contributor

@mzeis mzeis commented May 31, 2019

There was no security announcement, but Mirasvit_Helpdesk has a "possible XSS security issue" up to 1.5.14: https://mirasvit.com/doc/extension_helpdesk/current/changelog

Note that MageVulnDb may report an older version for Mirasvit_Helpdesk than you actually have installed because a new marketing version might not trigger a change in config.xml. For example, marketing version 1.5.11 has the XML config entry <version>1.0.36</version>.

I will update this issue when I know if 1.5.14 has a newer XML config version.

There was no security announcement, but Mirasvit_Helpdesk has a "possible XSS security issue" up to 1.5.14: https://mirasvit.com/doc/extension_helpdesk/current/changelog
@gwillem
Copy link
Collaborator

gwillem commented May 31, 2019

For example, marketing version 1.5.11 has the XML config entry 1.0.36.

😒

@rhoerr
Copy link
Collaborator

rhoerr commented May 31, 2019

I haven't seen that on M1--that's unfortunate. For what it's worth that's standard practice on M2 though, because any module.xml version change requires an explicit setup run even if schema is unchanged. The M2 plugin looks at composer version when available, though, so it's covered there.

If Mirasvit isn't updating their internal version number with releases, we should ask them to fix that.

@gwillem
Copy link
Collaborator

gwillem commented Jun 9, 2019

Fwiw, M1's Mirasvit_Seo uses the version from etc/config.xml as "database schema version", the reported code version is in Mirasvit/Seo/Helper/Code.php.

gwillem added a commit that referenced this pull request Jun 9, 2019
@gwillem
Copy link
Collaborator

gwillem commented Jun 9, 2019

I have asked Vladimir of Mirasvit to have a look at this discussion and temporarily disabled the Mirasvit extension listings (because they produced many false positives).

@volodymyr-d
Copy link

In our extensions, we don't change the version number in the file etc/config.xml with each release. We change the database version number only if we do changes in the database schema.

We have own version number which you can find the file file Mirasvit/Helpdesk/Helper/Code.php .
This file has the following format:

<?php
class Mirasvit_Helpdesk_Helper_Code extends Mirasvit_MstCore_Helper_Code
{
....
    protected $v = "1.5.16";
....
}

Variable $v is the current version of the package. This version number is automatically changed with each release. We refer to this version in our change logs.

The same approach is used for all our M1 extensions.

@gwillem
Copy link
Collaborator

gwillem commented Jun 14, 2019

@volodymyr-d thanks for replying. Would you consider using the standard (config.xml) version number as "main" version? Otherwise, tools such as n98-magerun and this one would have to make a custom parser, just for your extensions.

@volodymyr-d
Copy link

@gwillem currently we don't do many changes in M1 extensions. only bug fixes. so even if change our build system and workflow, it will not solve this problem for external tools.

@gwillem
Copy link
Collaborator

gwillem commented Jul 15, 2019

Anyone else got a clever idea how to handle this? I've been brewing without much luck :D

@rhoerr
Copy link
Collaborator

rhoerr commented Jul 15, 2019

Anyone else got a clever idea how to handle this? I've been brewing without much luck :D

I don't want to seem callous, but it seems to me this is the crux of the issue:

In our extensions, we don't change the version number in the file etc/config.xml with each release.

There are conventions and standards, and if vendors won't follow those, that's their problem. If they won't update their version number, they need to understand that this tool is going to report their module as vulnerable until they do.

--

I appreciate the logic in not updating the schema version unless there are in fact schema changes--that's standard practice in M2--but M2 provides a standard way of defining version number independent of that (composer), and M1 does not. Others can't be expected to support any arbitrary means a vendor might have of defining their own release version number, so for M1 the module version must be changed for each release.

It's either that or we define our own standard for release version independent of Magento's setup version (such as adding 'module_version' or such in the XML)--but I would peg the chance of adoption and success of such an effort at right about zero. If Mirasvit would actually use such a thing, though, that's something we could add support for in the tooling with relative ease, and that would avoid any vendor-specific code or dependencies.

@gwillem
Copy link
Collaborator

gwillem commented Jul 25, 2019

Perhaps Mirasvit will eventually change their mind. For now, let's merge this so the repo contains at least the most up to date information.

@gwillem gwillem merged commit 80ad7b6 into sansecio:master Jul 25, 2019
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.

4 participants