You have an error in your SQL syntax

Please, help me with this code

[greenfeel]

Can you help with this Error message. Thank you.

I receive this Error:
You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'WHERE slug='privacy_policy'' at line 1
WHERE slug='privacy_policy'

The code I use is here:
<?php
if(!defined('INC_FROM_INDEX'))
{
die('Direct access is not allowed!');
}
if(isset($post['submit']))
{
$query = "UPDATE pages SET ";
foreach($LANG['index'] as $l_id => $l)
{
$query .= "text_lang{$l_id}='".$_POST['text'][$l_id]."', ";
}
$query = substr($query);
$query .= " WHERE slug='" . $url->dirs[3] . "'";
$db->query($query);
header('Location: ?');
}
$query = "SELECT * FROM pages WHERE slug='" . $url->dirs[3] . "'";
$post = array_merge($post, $db->fetch($db->query($query)));
foreach($LANG['index'] as $l_id => $l)
{
$post['text'][$l_id] = $post["text_lang$l_id"];
}
$template -> assign('main_tpl', 'admin/system/pages.tpl');
?>

And here:
<?php
if(!defined('INC_FROM_INDEX'))
{
die('Direct access is not allowed!');
}
$text = $db->fetch($db->query("SELECT text_lang$LANG[id] AS text FROM pages WHERE slug='privacy_policy'"));
$template->assign('text', $text['text']);

$template->assign('background', $backgrounds['home']);
$template->assign('box_title', $t['terms']);
$template->assign('page_title', $t['terms_title']);
$template->assign('page_id', 'privacy_policy');
$template->assign('main_tpl', 'privacy_policy.tpl');

[DeeCee]

Kinda multiple problems in that code.

> $query = "UPDATE pages SET ";
> foreach($LANG['index'] as $l_id => $l)
> {
> $query .= "text_lang{$l_id}='".$_POST['text'][$l_id]."', ";
> }
> $query = substr($query);
> $query .= " WHERE slug='" . $url->dirs[3] . "'";
> $db->query($query);
> header('Location: ?');
> }

The combination of

 $query = "UPDATE pages SET ";
 $query .= "text_lang{$l_id}='".$_POST['text'][$l_id]."', ";
 $query .= " WHERE slug='" . $url->dirs[3] . "'";

Will always produce something like "UPDATE pages SET X1 = X2, WHERE slug = X3"
There is always a comma after X2, right before your 'WHERE'.

Also in your PHP what is the idea with "$query = substr($query);"?
PHp substr require minimum 2 parameters, so you should be seeing complaints like "PHP Warning: substr() expects at least 2 parameters"

[Dinkar]

Off topic:

Why are you using the following code?

if(!defined('INC_FROM_INDEX'))

[topr8]

"text_lang{$l_id}='".$_POST['text'][$l_id]."',

OT... you are just waiting for your db to be hacked by using POST data directly in queries without checking/cleaning them first

[greenfeel]

Thanks. I am working on a site, which is not made by me and I am doing my best to fix the errors. The site was OK, bevore a month and now it is inpossible to make changes in the administrative text - and I don`t know what to do with this code. I try with the comma, but nothing - I receive an error for that comma too.

I think that the problem is somewhere here:

$text = $db->fetch($db->query("SELECT text_lang$LANG[id] AS text FROM pages WHERE slug='privacy_policy'"));

The exact error is:
You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'WHERE slug='privacy_policy'' at line 1
WHERE slug='privacy_policy'


Can you help me and check this sintax, please.
Thank you.

[DeeCee]

To Dinkar:
The "if(!defined('INC_FROM_INDEX')) DIE" is probably the only good line of code in the whole extract.
The idea is to start each PHP file checking that it is not being called alone from some scam URL-call, but is used only by including into the larger system, after all environment has been set up (including here 'INC_FROM_INDEX', whatever that might to this code).
Systems such as Joomla use he same structure by starting each php included file (except for the normal entry index.php) with a line like

defined('_JEXEC') or die;

_JEXEC being defined by Joomla when loaded normally. Largely prevents from hackers calling on include files that should not be loaded on their own, but only included by the system itself.

To topr8:
100% right. :-) Thats why I mentioned that there are multiple problems in that code. Baaad code.

To greenfeel:
There is nothing wrong with the line

$text = $db->fetch($db->query("SELECT text_lang$LANG[id] AS text FROM pages WHERE slug='privacy_policy'")); 

in itself, other than you not providing any context.

The actual SQL in that line should work PROVIDED that the PHP part is OK, which we do not have.
Before the SQL is executed, the PHP piece "$LANG[id]" have to be evaluated by the interpreter to expand the string, and we do not know what the environment has set the array $LANG or the specific index "[id]" to at time of execution.. Without that we cannot know what the resulting SQL actually will end up looking like.

Either use a PHP debugger (which you likely don;t have/use) to step through it, or simply stick some echo's and print_r() calls in the code to see what the results actually look like.

[Dinkar]

>>text_lang$LANG[id]

Is this correct? Don't you need space between text_lang and $LANG[id]?



To DeeCee:

Thank you for detailed explanation. Very helpful.

I don't know Joomla; but I understand the reason for using the code. But....

Is this safe?

Largely prevents from hackers calling on include files that should not be loaded on their own, but only included by the system itself.

What if hacker use same code used in system's index file to define it and then call the include files? Is it not possible?

Again I would like to ask same question... is this safe?

[greenfeel]

everything is OK now.
My problem was here:

"$query = substr($query);"?

Thank you very much for the idea where to search it.

[DeeCee]

Dinkar,

Yes, a line like

defined('SOME_SYMBOL') or die( 'Restricted access' );

in the start of every include-file for fixed environment type code is quite safe.

Unless a hacker is physically on your system and can change files (in while case it has no meaning anyway), they cannot cheat that and call an include file directly. Since it is a PHP symbol define and not an environment variable, the only way they could "set it like if from index.php" would be to have added another PHP file to your system acting like your normal entry point (index.php normally), which then includes it.

Hence having each include file check its PHP defined symbol environment for correctness in some way similar to that statement prevents someone knowing the include file's URL path from calling it like for example

yoursite.net/includes/your_include_file.php

With "your_include_file.php" checking for an expected variable environment and "dieing" if not defined, all they get from web-calling on that file on its own by its URL is a "Restricted access" popping out. No further execution. So even for public source code where hackers can study the include files for useable stuff, this stops a lot of "false calls" on include files cold.

[Dinkar]

DeeCee,

Thank you for detailed information. I never tried it. I am using different system. I will try this in addition to it to make it more secure. Thanks :)

[StoutFiles]

It bothers me when people throw out a blob of code that they obviously didn't write and ask people to hunt through it finding all the errors. If you want help at least try writing it yourself.

[DeeCee]

StoutFiles,
I am in 100% agreement.
And sometimes a basic search on php.net should be warranted first.