Topic: To the dev that caught me...

[Note: This post will be slightly abstract and incomplete. The intended recipient will understand, and that is all I require. Should (s)he decide to divulge more information, then that is their decision.]


First off: Kudos for taking action so quickly. Most dont even notice, let alone it in the act...
Secondly: Excellent job in applying the correct fix, on the first try. A lot of the people I come across underestimate the simplicity but effectiveness of intval, thus, leaving their application open to other, more complex vulnerabilities.


Because of your swift response to the critical situation, I have decided to leave you (for now wink )


As a show of good faith, and no hard feelings; from one developer to another:
Some other issues that have not yet been addressed:

Path disclosure:
URL: http://imperialconflict.com/rankings_ov … re&g=8
By converting the "g" get param to an array, the server tosses a fatal error which exposes a server path.
(ie. ?type=topfamilies_score&g[]=8 )
Like wise: It would be a good idea to hide/remove This topic.


Potential Target:
Why is this page even live?
I mean... Really?
http://imperialconflict.com/lib/db.lib
This should be inaccessible. Deny it!


Errors Displayed: ***ERRORS DIRECTLY FROM THE DB DRIVER***
Honestly, this has me a little worried and second thinking my kindness of leaving.
But, I am an advocate of practising good security. As such, I feel like this is an important message to note: While it is fine and dandy to have errors displayed on a dev/local copy of the site... In NO instance should an error ever be shown on a production site.
There are no exceptions...
This include the fatal error above, but this MORE SO applies to errors from poorly written (and sanitized) queries.
Converting the id param to an array on http://imperialconflict.com/profile.php?id=1 will show you an error that comes directly from the driver. This should NEVER... Ever, EVER EVER be shown on a production site.
Not only is it an error (see above), but mysql errors show partial queries.
Please fix it.
(sidenote: There is also a SQLi target that relates to this. I didnt bother checking in depth)


The fix for the errors is a simple one. Check for isset and is_numeric/is_string and/or isset/empty intval/trim combo.
(ie.

if (isset($_GET['p'])&&is_numeric($_GET['p']) { do_stuff... }
if (isset($_GET['p'])&&!empty(trim($_GET['p']) { do_stuff... }
if (isset($_GET['p'])&&!empty(intval($_GET['p']) { do_stuff... }

)
That should eliminate the underlying problem. However the other issue is that errors are still shown. error_reporting(0).
You should be watching error logs anyways. All errors should be transparent to end users. This is how you provide a good user experience.
Lastly, use parametrized queries. I dont know how many devs work on this site, but someone needs to spend at least a few days converting the ORM over to use parameters. Not built queries.


Developer to developer, I am in hopes that you will respond maturely to this post.
But, In the event that you dont, I am withholding a (so far) single XSS potential.


I look forward to hearing a response. I will check back at some point in the future.
Perhaps then, we can be on the "same team" next time... wink


Regards

(ps: You should consider reporting to the punBB people that they shouldnt rely on poor, outdated code. Especially since they *JUST* released an update...
Im talking about "preg_replace(): The /e modifier is deprecated".
Can be reproduced by forcefully crashing a reply, with text inside, and reloading it.
parser.php on lines 59, 835, and 836)
(pps: Again with the error reporting. You can turn this off in the ini...)

2 (edited by exceed 14-Sep-2015 16:56:08)

Re: To the dev that caught me...

could there be any cheating with these vulnerabilities ?

Re: To the dev that caught me...

Individually, on their own, no.
But combined together, of course.
Once you have access to their database, with their current set up, you can (more or less) do what you would like.
*cough* Change the user, dev! Something that has MUCH less permissions. *cough*


From just a "cheating" standpoint, once you have access to the database, you find the tables that contain what you would like to modify, find the related data (for example, your money) and change it.


But my post goes much deeper than just cheating. There is much worse that could be had.

Re: To the dev that caught me...

Don't bother, he's not here to help us.

~*✠ ]PW[ Forever ✠*~

Re: To the dev that caught me...

He clearly helped enough to thwart the initial attack.

He may not be involved as much as you guys would want, but you don't give him enough credit for keeping things running.

Got a few bucks?  The Imperial Tip Jar is accepting contributions!

Re: To the dev that caught me...

He's a silent guardian...a watchful protector...

<KT|Away> I am the Trump of IC

Re: To the dev that caught me...

http://i.imgur.com/tzyp92L.jpg

Got a few bucks?  The Imperial Tip Jar is accepting contributions!

Re: To the dev that caught me...

i also had found a few holes in the system which will hopefully be closed soon hmm

but these already have a fix ticket on them.... after the attacks i thought id have a play and there is alot of gaps where server cdes can be accessed.... some more complex to get to then others!

War doesn't decide who is right, it decides who is left.