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

Detection of overly long lines with preview of last 100 characters of offenders. #8

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

philemerson
Copy link

I've added a few features to the code in order to address the problem of long lines and general speed of checking by limiting the process to just PHP files (this can easily be changed in the constant).

Here's the summary:
Added option to disable sending of email
Added option to display results as console output
Added optional checking for long lines (with define to select trigger length)
Added display of last 100 characters of long lines to help identify false positives
Added filtering of input files so PDFs, JPGs etc can be skipped

Added option to display results as console output
Added optional checking for long lines (with define to select trigger length)
Added display of last 100 characters of long lines to help identify false positives
Added filtering of input files so PDFs, JPGs etc can be skipped
@philemerson philemerson changed the title Added ability to check for long files with preview of last 100 chars of file as well as filtering the files actually check (e.g. only PHP files instead of every file in the path) Detection of overly long lines with preview of last 100 characters of offenders. Feb 19, 2016
@hazrpg
Copy link

hazrpg commented Feb 22, 2016

This seems to fix issue #7.

@coliff
Copy link

coliff commented Mar 16, 2017

This is a great PR with many useful additions. It's worked well for me. I have a few suggestions though;

  • Increase the default 'LONG_LINE_THRESHOLD' to something much longer - right now it gives a lot of false positives (especially with WordPress)
  • In the filetypes in 'FILES_TO_MATCH' you could add PHP3, PHP7 and PHPS. I realise they aren't so common, but can do no harm.

@mikestowe
Copy link
Owner

Looks good! Let me just run some tests to make sure I'm not missing anything and then will merge it in!

@clinttepe
Copy link

There seems to be an issue/typo on line 61 - the 2nd and 3rd parameters for str_pad() are reversed (or at least that's what fixed the error I was getting). Here is what worked for me with PHP 5.5.9 and 5.6.30:
$this->infected_files[] = $file."\n".str_pad('base64/eval found',30,' ',STR_PAD_LEFT)."\n";
Still testing this version of the scanner combined with @coliff's suggestions. Really like the added features!

@clinttepe
Copy link

str_pad() issue also present with PHP 7.0.18
@mikestowe is there anything I can do to help test and push this PR through?
I am working on some ideas to improve detection accuracy with the long line scan.

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.

5 participants