There's some buzz on twitter that Ninjaboard got an SQL injection vulnerability.
Here's the first tweet that started the buzz:
New Exploit [dos] - Joomla Ninjaboard Component (com_ninjademo) SQL Injection Vulnerability: http://bit.ly/aa5CnQ
Now, first off. The component being referenced is actually com_ninjademo.
NinjaDemo has nothing to do with Ninjaboard.
NinjaDemo is not even released, and it likely never will be either. It's an in-house extension we're using to quickly setup demo links, as seen on the Ninjaboard Demo site
The security vulnerability disclosed is that if you pass something other than an ID into the link, like this: http://ninjaforge.com/ninjaboard-demo/index.php?option=com_ninjademo&view=demo&id=[sql] then an error happens because the generated SQL query gets a syntax error.
However that's terribly wrong, follow that link and you'll get this error:
Warning: Invalid argument supplied for foreach() in tmpl://components/com_ninjademo/views/demo/tmpl/default.php on line 11
Since NinjaDemos source is unavailable, I'll show you the code in that file here so you know what's happening:
<? foreach ($demo->links['href'] as $i => $link) : ?>
We store each demo link in a JSON encoded object. Like this:
{
"link": ["#", "#"],
"title": ["Foo", "Bar"],
"image": ["foo.png", "bar.png"]
}
The reason that warning happens, is simply because if you don't supply the right id, no JSON data is fetched, so this part of our layout will fail.
There is absolutely no SQL injection happening. We take great pride in security, and NinjaDemo is also using Nooku Framework. So the id variable is sanitized multiple times before the sql query is generated.
- URL variable is fetched using KRequest::get('get.id', 'int'), so it's sanitized as an integer.
- The id is put as a model state, and the model state is configured as
$this->_state->insert('id', 'int') in the model. Meaning it's sanitized as an integer for the second time.
- Before the database query is generated, Nooku Framework will filter each column according to table metadata. The id is a bigint type, so it's sanitized as an integer for the 3rd time during the generation of the SQL query.
So it's pretty obvious that there is absolutely no SQL injection going on here.
"Then what is going on?"
Like I said earlier, the fault is that if the supplied ID don't exist, no JSON data is sent to the view. So when we do the foreach loop, $demo->links['href'] don't exist, as $demo->links wont hold any data, so we get a PHP warning.
What should be done in our case, is to check that the JSON data exists before trying to loop it.
That's all there is to it. I would appreciate it if the people who reported this "vulnerability" in the future contact us before making it public, so we get a chance to prevent a faulty report like this to be made public.
After all, it's common decency to alert the author before making reports public.
Especially if they think it's an piece of software that's used on many sites (they did think the extension was Ninjaboard), as it's important to be able to fix vulnerabilities so users can update their sites before they could be attacked.
XML Feeds