MyBB Community Forums

yes, already doing all that in the new version

---

and the new version of MyShowcase (2.2) was posted to the mods site a little bit ago.

(2011-11-02, 12:59 PM)TheGarfield Wrote: [ -> ]Actually it is a dangerous SQL injection,

Imagine if I'm the vistor and I enter in the posthash field something like:
0' OR 1=1; DELETE * FROM 'mybb_posts

The query now is:

$query = $db->simple_select("myshowcase_attachments", "*", "posthash = '0' OR 1=1; DELETE * FROM 'mybb_posts'");

OR MORE CLEARLY, The query now is:

SELECT * FROM myshowcase_attachments WHERE posthash = '0' OR 1=1; DELETE * FROM 'mybb_posts'

I deleted ALL the posts in your forum!!

The PHP-MySQL driver doesn't allow multiple queries, which is a good thing for security, so unfortunately, your suggestion won't work.
It's still quite exploitable, though I do wonder if anyone here is able to figure anything out...

(2011-11-02, 02:36 PM)pavemen Wrote: [ -> ]thanks for the not so subtle report of a vulnerability, but its already been corrected in my current code that I am almost done with.

Not like it wasn't subtle or anything to begin with. I just opened the first .php file, and the first instance of "$db" gave me that.
Also, why you're doing a SELECT * query for just a rowcount is beyond me.
But if you need me to troll you any more, just ask!

(2011-11-02, 11:20 PM)Yumi Wrote: [ -> ]Not like it wasn't subtle or anything to begin with. I just opened the first .php file, and the first instance of "$db" gave me that.
Also, why you're doing a SELECT * query for just a rowcount is beyond me.
But if you need me to troll you any more, just ask!

well, its all been corrected. I went through every file, looked for the queries and back tracked all the data that went into them to make sure they are wrapped in intval() or $db->escape_string() when coming from an $mybb->input (directly or assigned from).

as for the query in question, i am not sure why i am doing that query the way it is, though I think I was originally using all of the data farther down in the code, but changed something and did not convert to a simpler COUNT() query. I dunno, maybe just a something I overlooked.

(2011-11-02, 11:48 PM)pavemen Wrote: [ -> ]well, its all been corrected. I went through every file, looked for the queries and back tracked all the data that went into them to make sure they are wrapped in intval() or $db->escape_string() when coming from an $mybb->input (directly or assigned from).

You should be checking all variables, not just ones coming from input.
Just because something doesn't come directly from an input, doesn't mean it's inexploitable (sorry for the triple negative).

(2011-11-02, 04:56 PM)labrocca Wrote: [ -> ]Also checks for post code and input method as POST are the full monty imho for MyBB security.

if($mybb->input['posthash'] && $mybb->input_method="post")

verify_post_check($mybb->input['my_post_key']);

if($mybb->input['posthash'] && $mybb->request_method == "post")
{
     // Code

(2011-11-03, 06:35 AM)Yumi Wrote: [ -> ]

(2011-11-02, 11:48 PM)pavemen Wrote: [ -> ]well, its all been corrected. I went through every file, looked for the queries and back tracked all the data that went into them to make sure they are wrapped in intval() or $db->escape_string() when coming from an $mybb->input (directly or assigned from).

You should be checking all variables, not just ones coming from input.
Just because something doesn't come directly from an input, doesn't mean it's inexploitable (sorry for the triple negative).

i did check them all, but all other variables I am using are directly assigned by me before the query or pulled from a query result and strings are escaped where used. so as far as I know, the non ACP pages I have are now clean. I did not put as much effort into ACP side since if someone is there, they can do other damage

That's just sloppy coding. You should be escaping in all the appropriate places.
Not escaping isn't just about security, it's about correct functionality.

(arguably MyBB's framework can be blamed for not automatically escaping stuff, but you're stuck with this unfortunately so you've got to play by the rules)

The only string that was not escaped was posthash, which I wrongly assumed was handled automatically my MyBB and since I know that a correct value does not contain bad characters I let it slide.

There are a few integers that I missed forcing the type on but all other expected strings were escaped, in ACP or not as I know an odd character can break the DB save or the output.

So I have fixed the issue and submitted the updated code within a few hours of you guys letting me know. We can get back to the original topic in general now. :-)

Be aware that like 90+% of security vulnerabilities come from bad assumptions.

Always use mysql_real_escape_string(); and you are secure!
No one can inject you