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

File integrity gives bad advice on symlinks #16551

Open
gerard76 opened this issue Oct 8, 2020 · 7 comments · May be fixed by #22723
Open

File integrity gives bad advice on symlinks #16551

gerard76 opened this issue Oct 8, 2020 · 7 comments · May be fixed by #22723
Labels
Bug For errors / faults / flaws / inconsistencies etc. Help wanted Beginner friendly issues or issues where we'd highly appreciate community's help and involvement.

Comments

@gerard76
Copy link

gerard76 commented Oct 8, 2020

I added symlinks to matomo.js and matomo.php because matomo is a keyword that is blocked by some blockers.

File integrity is correct in pointing out they should not be there, but the solution it proposes is wrong.
Screenshot 2020-10-08 at 13 56 40

Matomo 3.14.1

2020-10-10 - edit fixed typo

@Findus23 Findus23 transferred this issue from matomo-org/matomo-package Oct 8, 2020
@Findus23 Findus23 added the Help wanted Beginner friendly issues or issues where we'd highly appreciate community's help and involvement. label Oct 8, 2020
@tsteur
Copy link
Member

tsteur commented Oct 8, 2020

Hi @gerard76

I think this is actually expected behaviour and the solution be to mark these files as "to be ignored" by creating a file config/config.php like with content like this:

<?php
return array(
    'fileintegrity.ignore' => DI\add(array(
        'w.js',
        'w.php',
    ))
);

If someone else can confirm this is expected behaviour then this could be put in an FAQ.

@Findus23
Copy link
Member

Findus23 commented Oct 8, 2020

I think the bug reported isn't that those files are shown, but that the check recommends deleting the original files instead of the "incorrect" symlinks.

If someone wants to fix this, they are welcome to submit a Pull Request.

@tsteur
Copy link
Member

tsteur commented Oct 8, 2020

👍 didn't notice there was a different message. For this particular case it would actually remove those files by adding the config but of course the actual bug remains which is the use of realpath in https://github.com/matomo-org/matomo/blob/4.0.0-b2/core/FileIntegrity.php#L75

@tsteur tsteur added the Bug For errors / faults / flaws / inconsistencies etc. label Oct 8, 2020
@tsteur tsteur added this to the Priority Backlog (Help wanted) milestone Oct 8, 2020
@aureq
Copy link

aureq commented Oct 9, 2020

Hi @gerard76 Instead of using symlinks, may I suggest you to use aliasing instead? This way you won't need any symlinks and you should be able to achieve what you want.

@gerard76
Copy link
Author

Sure you may. I have no problems with the symlinks though. Just reporting a bug.

@narfk
Copy link

narfk commented Nov 26, 2020

Another solution (Apache, .htaccess)

<IfModule rewrite_module>
	RewriteEngine On
	RewriteRule ^/w.(js|php)$ /piwik/piwik.$1 [NC,L]
</IfModule>

@danielpunkass
Copy link
Contributor

danielpunkass commented Oct 29, 2024

I ran into this issue today and, indeed, it led to my inadvertently deleting a critical matomo source file. As @tsteur mentioned above the issue is caused because the paths are passed through realpath before being presented to the user. I think that a fine solution is to remove the realpath calls, because the paths, all being returned ultimately from glob calls, should be sufficiently canonical to make sense to the user, and to be executable on the command line. Can anybody think of a scenario where not calling realpath would be problematic? I'm going to submit a PR that removes these.

Tracing the history of the realpath calls, they go back to c8797e6, associated with #11123. Maybe @mattab can comment on whether there was a particular concern when the realpath calls were originally used.

If anybody is anxious about removing the realpath calls another option might be to run a final check before presenting a path to the user, to ensure it's not actually a path that IS part of the manifest. But with this approach the integrity checker would neglect to show the user the unexpected symlink file in the directory.

danielpunkass added a commit to danielpunkass/matomo that referenced this issue Oct 29, 2024
… user. Calling realpath on a symlink that was found in the tree, for example, causes Matomo to suggest to the user deleting the target of the symlink. At best this would be a nuisance, and at worst it could be catastrophic (if the user kept a symlink to files outside of Matomo's directory, for example). Fixes matomo-org#16551.
@danielpunkass danielpunkass linked a pull request Oct 29, 2024 that will close this issue
11 tasks
danielpunkass added a commit to danielpunkass/matomo that referenced this issue Oct 29, 2024
… user. Calling realpath on a symlink that was found in the tree, for example, causes Matomo to suggest to the user deleting the target of the symlink. At best this would be a nuisance, and at worst it could be catastrophic (if the user kept a symlink to files outside of Matomo's directory, for example). Fixes matomo-org#16551.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc. Help wanted Beginner friendly issues or issues where we'd highly appreciate community's help and involvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants