About: Scott Paley

Jason –

The s2member-pro Veracode report shows 2 code injection issues at the file s2member-pro\includes\classes\login-widget.inc.php.

It uses $_SERVER[“REQUEST_URI”] in raw form to echo it out in HTML (into an field and into a link).

The value of $_SERVER variables can’t be trusted, as it can be tampered by the end user.

This is why the WP core uses the esc_url() function whenever WP needs to use REQUEST_URI.
You’re using esc_attr instead, which doesn’t seem to provide enough URL cleaning, and thus open this input for XSS.

Could you comment on this?

Just to be clear about the scan. We uploaded all of our source code to Veracode, which has some method of analyzing all the code and finding security holes. The output of that was what raised our alarms.

The dynamic scan we’ll run over the weekend is more like Selenium. It’ll actually mimic a user trying to break the site from the front-end. I’ll be happy to share those results as well.

And of course, the manual scan is literally a human being trying to hack the site via methods like SQL injection, XSS, or other nefarious means.

Thanks Jason,

I just sent the reports. I’m sure you take security very seriously, which is why I brought this to your attention immediately. I’d be thrilled if you could prove to me these are all false positives, but if not, we really need these fixed urgently (as I’m sure do many of your users.)

Thanks – will try that.

Yes, I agree with you. I’m hoping it’s all just false positives. But… the network won’t sign off on the website until we can either prove it’s a false positive, or if it’s not, that it gets fixed.

And if it really IS an issue, it’s a pretty major issue that will affect all s2member websites and really should be fixed urgently.

David – I’m not certain. It’s a static scan, not done manually. It’s entirely possible it’s found some “false positives.”

That said, this is certainly an urgent concern for us and I’d imagine for s2member as well.