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

remove WARNING from php-errors.data #1343

Merged

Conversation

Projects
None yet
3 participants
@theMiddleBlue
Copy link
Collaborator

commented Apr 9, 2019

referring to #1182 this PR removes <b>Warning</b>: from php-errors.data. All PHP warning messages already match 953100.

@dune73 dune73 added the Needs action label May 5, 2019

@dune73

This comment has been minimized.

Copy link
Collaborator

commented May 5, 2019

Tests are failing. Can you fix?

@theMiddleBlue

This comment has been minimized.

Copy link
Collaborator Author

commented May 7, 2019

uhm.. this is really odd. I haven't change anything on the rule file. Can anyone help me to understand why it fails?

If I'm not wrong, the first failed test is 920240-4 and it seems to work correctly:

$ cat test920240-4.txt 
<SOAP-ENV:Envelope xmlns:SOAP-ENV="http://schemas.xmlsoap.org/soap/envelope/" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
  <SOAP-ENV:Body>
      <xkms:StatusRequest xmlns:xkms="http://www.w3.org/2002/03/xkms#" Id="_6ee48478-fdd6-4d7d-b1bf-e7b4c3254659" ResponseId="_c1c36bf-f962-4aea-bfbd-07ed58468c9b" Service="http://www.soapclient.com/xml/xkms2">
      <xkms:ResponseMechanism>http://www.w3.org/2002/03/xkms#Pending</xkms:ResponseMechanism>
      <xkms:RespondWith>%1Gwww.attack.org</xkms:RespondWith>
      </xkms:StatusRequest>
  </SOAP-ENV:Body>
</SOAP-ENV:Envelope>

$ curl -v -XPOST -d @./test920240-4.txt \
  -H 'User-Agent: ModSecurity CRS 3 Tests' \
  -H 'Accept: text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/png,*/*;q=0.5' \
  -H 'Accept-Language: en-us,en;q=0.5' \
  -H 'Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7' \
  -H 'Keep-Alive: 300' \
  -H 'Proxy-Connection: keep-alive' \
  -H 'Content-Type: text/xml' \
  'http://localhost'

matched rules:

  • [920240] URL Encoding Abuse Attack Attempt
  • [920272] Invalid character in request (outside of printable chars below ascii 127)
  • [920273] Invalid character in request (outside of very strict set)
  • [942440] SQL Comment Sequence Detected.
  • [942432] Restricted SQL Character Anomaly Detection (args): # of special characters exceeded (2)
  • [949110] Inbound Anomaly Score Exceeded (Total Score: 24)
  • [980130] Inbound Anomaly Score Exceeded (Total Inbound Score: 24 - SQLI=8,XSS=0,RFI=0,LFI=0,RCE=0,PHPI=0,HTTP=0,SESS=0): Restricted SQL Character Anomaly Detection (args): # of special characters exceeded (2); individual paranoia level scores: 6, 5, 5, 8
@dune73

This comment has been minimized.

Copy link
Collaborator

commented May 7, 2019

Something similar happened to me before with Travis. I solved it with reapplying the changeset and force pushing it into the repository. (All with the support of @studersi). Here is what we did:

	git clone git@github.com:SpiderLabs/owasp-modsecurity-crs.git crs-pr-1392
	cd crs-pr-1392
	git fetch --all
	git remote show origin
	git remote add dune73 git@github.com:dune73/owasp-modsecurity-crs.git
	git fetch --all 
	git checkout remotes/dune73/rule-vs-x-up-devcap-post-charset
	git checkout -b rule-vs-x-up-devcap-post-charset
	git pull origin
	git checkout rule-vs-x-up-devcap-post-charset
	git stash
	git rebase v3.2/dev
	git push dune73 -f

You might not need all of this.

When you push this, a new travis run will be initiated. That did the job for me.

ŮPDATE: I just checked: The tests failing are exactly the ones that failed in my PR as well. Apply the procedure above and you'll be fine.

@theMiddleBlue theMiddleBlue force-pushed the theMiddleBlue:php-warning-fp-1182 branch from ff0dcda to 25345ac May 7, 2019

@theMiddleBlue

This comment has been minimized.

Copy link
Collaborator Author

commented May 7, 2019

thanks @dune73, I couldn't have done it without you. Travis seems happy

@csanders-git

This comment has been minimized.

Copy link
Collaborator

commented May 7, 2019

So it looks like this is ready for a merge, yeah?

@theMiddleBlue

This comment has been minimized.

Copy link
Collaborator Author

commented May 7, 2019

Yes! Now I've permission for merge and I'm not afraid to use it :)

@dune73 dune73 removed the Needs action label May 7, 2019

@dune73

This comment has been minimized.

Copy link
Collaborator

commented May 7, 2019

Then you should go ahead and merge it.

(-> Please use the pulldown menu on the merge button and pick "Squash and merge" so we get a cleaner commit history.

@theMiddleBlue theMiddleBlue merged commit 87dc5a5 into SpiderLabs:v3.2/dev May 7, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.