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

Symlink detection, warning and scanning are not working as intended #183

Closed
ewodrich opened this issue Dec 4, 2023 · 4 comments · Fixed by #185, #190, #196 or #150
Closed

Symlink detection, warning and scanning are not working as intended #183

ewodrich opened this issue Dec 4, 2023 · 4 comments · Fixed by #185, #190, #196 or #150
Assignees
Labels
bug Something isn't working dev-complete Development work to resolve this issue is complete qa-passed QA has tested and confirmed the fix for this issue subcommand:malware-scan Related to the malware-scan subcommand
Milestone

Comments

@ewodrich
Copy link

ewodrich commented Dec 4, 2023

See case #25 and #74 which addressed symlinks earlier. It appears the message and handling of symlinks was altered possibly when addressing symlinks pointing to themselves in another case. Recursive symlinks are now receiving the message "symlink pointing to itself . . . " rather than the original message of "Recursive symlink detected at /path/being/scanned"

Example:

  • The path I’m scanning is at /var/www/html/e-vm.com/public_html/wordpress
  • The symlink points to the following directory/var/www/html/e-vm.com/public_html/mlwr/9997 which is outside of the “wordpress” directory
  • The message I’m getting is Symlink pointing to itself detected at /var/www/html/e-vm.com/public_html/wordpress/mlw9997

In addition to the warning message, the symlinked directory is not being scanned for malware.

@ewodrich ewodrich added bug Something isn't working subcommand:malware-scan Related to the malware-scan subcommand labels Dec 4, 2023
@ewodrich ewodrich added this to the v2.1.0 milestone Dec 4, 2023
@akenion akenion self-assigned this Dec 5, 2023
@akenion akenion added the dev-complete Development work to resolve this issue is complete label Dec 5, 2023
@akenion
Copy link
Contributor

akenion commented Dec 5, 2023

The problematic check was intended to detect symlinks pointing to themselves using the Python built-in os.path.samefile function. This function resolves symlinks and hence always results in a symlink and its real path being equivalent. This has been corrected by checking the type of the file prior to invoking os.path.samefile.

@akenion akenion added the qa-ready Issue is ready for QA and included in the most recent release candidate label Dec 6, 2023
@davidnuzik davidnuzik self-assigned this Dec 6, 2023
@ewodrich
Copy link
Author

ewodrich commented Dec 6, 2023

Re-opening for the following issue where regular (ie non-symlinked) files are being interpreted as symlinks when files are scanned with the --read-stdin option. Ex: find . -name '*.html' -print0 | wordfence malware-scan --read-stdin -a

templates directory:

-rw-r--r--@ 1 Esthe staff 1673 Oct 30 09:59 archive.html
-rw-r--r--@ 1 Esthe staff 1815 Oct 30 09:59 search.html
-rw-r--r--@ 1 Esthe staff 1502 Oct 30 09:59 blog-alternative.html
-rw-r--r--@ 1 Esthe staff 60 Oct 30 09:59 blank.html
-rw-r--r--@ 1 Esthe staff 318 Oct 30 09:59 404.html
-rw-r--r--@ 1 Esthe staff 2063 Oct 30 09:59 home.html
-rw-r--r--@ 1 Esthe staff 1366 Oct 30 09:59 index.html
-rw-r--r--@ 1 Esthe staff 890 Oct 30 09:59 page.html
-rw-r--r--@ 1 Esthe staff 939 Oct 30 09:59 single.html

Scanning path: ./wp-content/themes/twentytwentythree/templates/archive.html
Symlink pointing to itself detected at ./wp-content/themes/twentytwentythree/parts/comments.html
Symlink pointing to itself detected at ./wp-content/themes/twentytwentythree/parts/footer.html
Symlink pointing to itself detected at ./wp-content/themes/twentytwentythree/parts/header.html
Symlink pointing to itself detected at ./wp-content/themes/twentytwentythree/templates/single.html
Symlink pointing to itself detected at ./wp-content/themes/twentytwentythree/templates/page.html
Symlink pointing to itself detected at ./wp-content/themes/twentytwentythree/templates/index.html
Symlink pointing to itself detected at ./wp-content/themes/twentytwentythree/templates/home.html
Symlink pointing to itself detected at ./wp-content/themes/twentytwentythree/templates/404.html
Symlink pointing to itself detected at ./wp-content/themes/twentytwentythree/templates/blank.html
Symlink pointing to itself detected at ./wp-content/themes/twentytwentythree/templates/blog-alternative.html
Symlink pointing to itself detected at ./wp-content/themes/twentytwentythree/templates/search.html
Symlink pointing to itself detected at ./wp-content/themes/twentytwentythree/templates/archive.html

@ewodrich ewodrich removed qa-ready Issue is ready for QA and included in the most recent release candidate dev-complete Development work to resolve this issue is complete labels Dec 6, 2023
@ewodrich ewodrich self-assigned this Dec 6, 2023
@davidnuzik davidnuzik removed their assignment Dec 6, 2023
@akenion akenion linked a pull request Dec 7, 2023 that will close this issue
@akenion akenion added dev-complete Development work to resolve this issue is complete qa-ready Issue is ready for QA and included in the most recent release candidate and removed qa-ready Issue is ready for QA and included in the most recent release candidate labels Dec 7, 2023
@davidnuzik
Copy link

I'm still observing symlinks being followed repeatedly with malware-scan in rc8:

v2.1.0rc8 12/11/23

Steps:

  1. Set your current working directory to a wordpress install.
  2. Create a soft link via ln -s . points-to-self
  3. Initiate a scan with rc8 -- for example a malware-scan like wordfence malware-scan .
  4. Observe the symlink gets followed again even after it was already prior (and again and again)
    (my symlink is called "demosite" in the screenshot below and indeed points to itself demosite -> ./)
    image

Other scenarios with same result below. I think these make sense (the issue should not happen but the cause makes sense here too) -- this is perhaps the logic to check/search for other wordpress installs at the current dir or lower down:

  • Follow steps above but replace step 2 instead with command ln -s .. parent-link
  • " " " instead with command ln -s home/david/malware-sets-for-testing-cli/ 3-dirs-up
  • " " " instead with command ln -s ../../../ 3-dirs-up-relative

Scenarios I confirmed do not result in trying to infinitely follow a symlink:

  • Follow steps above but replace step 2 instead with command ln -s /home/david/malware-sets-for-testing-cli/demohackedsite/ other-install (just points to another install outside designated scanned path, sharing parent folder, but with no additional installs below this linked path "malware-sets-for-testing-cli" -- I think this makes sense that this doesn't cause the issue but posting here for posterity)

I am NOT seeing this behavior with the vuln-scan subcommand (simply wordfence vuln-scan .) after the change via #194

@davidnuzik davidnuzik self-assigned this Dec 12, 2023
@davidnuzik davidnuzik removed qa-ready Issue is ready for QA and included in the most recent release candidate dev-complete Development work to resolve this issue is complete labels Dec 12, 2023
@akenion akenion linked a pull request Dec 12, 2023 that will close this issue
@akenion akenion added the dev-complete Development work to resolve this issue is complete label Dec 12, 2023
@davidnuzik davidnuzik added the qa-ready Issue is ready for QA and included in the most recent release candidate label Dec 12, 2023
@davidnuzik
Copy link

v2.1.0rc9 12/12/23

SUMMARY:
QA validation PASSED. I can confirm this is all working correctly now :)
I confirmed that all points outlined in the issue description are now resolved.
I further confirmed that passing files to scan in via stdin (such as via find . -name '*.html' -print0 | wordfence malware-scan --read-stdin -a for example) now works as expected -- those files fed in via stdin are not treated like symlinks anymore.

REPRODUCTION AND VALIDATION STEPS

  1. REPRODUCE: Using v2.1.0rc3 and with a wordpress install, and within it a symlink to another wordpress install that is outside in a different folder (like Esther described in the issue summary).
  2. REPRODUCE: Using the same RC, execute a malware-scan on that folder -- I successfully reproduced the issue where the symlink is not followed and the symlinked path is not scanned.
  3. REPRODUCE: Now, still using the same RC, execute the following to pass html files to scan into wordfence cli: find . -name '*.html' -print0 | wordfence malware-scan --no-verbose --purge-cache --read-stdin -a in order to try to reproduce the issue Esther reported more recently where regular files were being detected as symlinks. I successfully reproduced the issue where each file passed in via stdin is reported as a symlink pointing to itself.

  1. VALIDATE: Now, using v2.1.0rc9 and with a wordpress install, and within it a symlink to another wordpress install that is outside in a different folder (like Esther described in the issue summary).
  2. VALIDATE: Still with RC9, Execute a malware-scan on that folder -- I successfully validated that the issue is resolved meaning that both the designated folder (main wordpress install) AND the symlinked 2nd wordpress install get scanned. Also note I have other symlinks pointing to themselves (same folder), parent dir, etc and they are not followed and are correctly detected as Symlink loops. My valid symlink to the 2nd wordpress install is correctly NOT detected as a Symlink loop. This verifies the main/initially reported issue is resolved and that related issues are not occurring (to check for regression on the mentioned issues in the issue description)
  3. VALIDATE: Now, still with RC9, Execute a malware-scan using piped input like find . -name '*.html' -print0 | wordfence malware-scan --no-verbose --purge-cache --read-stdin -a -- I successfully validated -- the wordfence CLI now correctly scans each piped in .html file without any issues. I do not see any warnings/errors about symlinks.

NOTES:
Additionally, I have expanded on our test coverage in our internal regression test doc so we do checks like these going forward earlier in the release cycle.

@davidnuzik davidnuzik added qa-passed QA has tested and confirmed the fix for this issue and removed qa-ready Issue is ready for QA and included in the most recent release candidate labels Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dev-complete Development work to resolve this issue is complete qa-passed QA has tested and confirmed the fix for this issue subcommand:malware-scan Related to the malware-scan subcommand
Projects
None yet
3 participants