Hello,

I have a site with 7 pages, on each of which users answer 4 questions.
The answers are submitted using AJAX and the php code below into a
MySQL database.

There is a possibility that the site may get mentioned on the radio
which could result in a high number of users accessing the site in a
short period of time.

Are there are any precautions I need to take re the adding of the data
to the database? Is the code below adequate?

Thanks

Geoff

@require(dirname(__FILE__) . '/../../../config/config.php');

$result1 = $_GET['answer1'];
$result2 = $_GET['answer2'];
$result3 = $_GET['answer3'];
$result4 = $_GET['answer4'];

mysql_connect($conf['sql']['host'], $conf['sql']['user'],
$conf['sql']['pass']) or die(mysql_error());
mysql_select_db($conf['sql']['db']) or die(mysql_error());

mysql_query("INSERT INTO mytable (answer1,answer2,answer3,answer4)
VALUES ('$result1','$result2','$result3','$result4')");

Geoff Cox wrote:
Impossible to tell from what you have. But it looks like you have done
absolutely no validation of the input data. The result can be a
complete destruction of your database - or worse. Google for "SQL
injection.

--
==================
Remove the "x" from my email address
Jerry Stuckle
JDS Computer Training Corp.
jstucklex (AT) attglobal (DOT) net
==================

On Sun, 04 May 2008 21:31:03 -0400, Jerry Stuckle
<jstucklex (AT) attglobal (DOT) net> wrote:

Jerry,

I have perhaps over simplified above - in fact only in one case is the
user asked to type in data - in the other cases it's a matter of
clicking on one of two images to give a response.

The box one does check for a number between 0 and 8

var number_check = number_given;
if ( (number_check >7) || (number_check < 1) ) {
alert("Tthe number must be in the range 1 to 7!");
} else
if (isNaN(number_check)) {
alert("Please enter a valid number");
} else {
sendGroup1Lab1(number_check);
}

is any further validation needed?

Cheers

Geoff

On Sun, 04 May 2008 21:31:03 -0400, Jerry Stuckle
<jstucklex (AT) attglobal (DOT) net> wrote:

Jerry,

where the user is typing a number into a box

var number_check = number_given;
if ( (number_check >7) || (number_check < 1) ) {
alert("The number must be in the range 1 to 7!");
} else
if (isNaN(number_check)) {
alert("Please enter a valid number");
} else {
sendGroup1Lab1(number_check);
}


I have changed (***) the php to

$result1 = $_GET['answer1'];
$result2 = $_GET['answer2'];
$result3 = $_GET['answer3'];
$result4 = $_GET['answer4'];

mysql_connect($conf['sql']['host'], $conf['sql']['user'],
$conf['sql']['pass']) or die(mysql_error());
mysql_select_db($conf['sql']['db']) or die(mysql_error());

$result4 = mysql_real_escape_string($_GET['favorite']); ***

mysql_query("INSERT INTO mytable (answer1,answer2,answer3,answer4)
VALUES ('$result1','$result2','$result3','$result4')");

I see that mysql_real_escape_string can only be used after connecting
to the database?

Is the above safer?

Cheers

Geoff

The wise Geoff Cox enlightened me with:
And what if the user crafts his own http response? You don't check the
data he is giving you, so you might be in trouble. sprintf's and/or
mysql_escape_string is your friend.

Mark
--
Terantula - Industrial Strength Open Source - http://www.terantula.com/
Projects and administration - +31 6 5140 5160

On Mon, 05 May 2008 01:38:23 +0200, Geoff Cox <gcox (AT) freeuk (DOT) notcom> wrote:
At least provide a more elegant solution then to 'die' with the database
message. Something like: include('sorrythesiteistobusyreturnlater.html');
exit; would be better.

Alterations based on a GET? Unless for logging purposes, I'd change that
to a POST.
--
Rik Wasmus

On Mon, 5 May 2008 09:14:40 +0200, Mark Huizer
<xaa+news_comp.databases.mysql (AT) dohd (DOT) org> wrote:


Mark,

I have now added mysql_escape_string to all the php files!

Cheers

Geoff

On Mon, 05 May 2008 09:43:14 +0200, "Rik Wasmus"
<luiheidsgoeroe (AT) hotmail (DOT) com> wrote:

Thanks Rik for both suggestions.

Cheers

geoff

On Mon, 05 May 2008 07:16:35 +0100, Geoff Cox wrote:

You must _never_ rely on client-side validation! As far as the
server-side script is concerned you should at least assume the user
switched JavaScript off (not to mention intended malformed values).

If those values are supposed to be numbers you should at least
ensure that:

$result1 = $_GET['answer1'] * 1;
$result2 = $_GET['answer2'] * 1;
$result3 = $_GET['answer3'] * 1;
$result4 = $_GET['answer4'] * 1;

In case not all digits are allowed (as your JavaScript fragment
suggests) a RegEx based test seems appropriate:

$hits = array();
$_GET['answer1'] = (isset($_GET['answer1'])
&& preg_match('|([1-7]+)|', $_GET['answer1'], $hits))
? $hits[1] * 1
: 0; // map invalid values to zero
$_GET['answer2'] = (isset($_GET['answer2'])
&& preg_match('|([1-7]+)|', $_GET['answer2'], $hits))
? $hits[1] * 1
: 0; // map invalid values to zero
[...]

(There's no need for those "$result" variables: Why keep the same value
in memory multiple times?) Do _not_ assume all the expected CGI
arguments are there actually but always check that.

What's "favorite" supposed to be? A string? A number (real or integer)?
How do you validate that?

The single quote are needed for string values, while those "$resultX"
variables are integers. Hence you could omit the single quotes.

Yes. However, id you'd validate the user provided values there's not
much need for such a call anyway. And escaping possibly malicious
values might avoid some SQL injection problem but it does _not_ avoid
your tables being filled with useless data.

Further improvement could be gained by not using the tables directly
but calling an stored procedure which could implement additional
validation. But that's another topic and probably oversized for
your application.

However, you should switch from HTTP/GET to HTTP/POST. While that's
not a safety net as such at least it makes it a _little_ harder to
fake the CGI arguments.

--
Matthias
/"\
\ / ASCII RIBBON CAMPAIGN - AGAINST HTML MAIL
X - AGAINST Microsoft ATTACHMENTS
/ \

On Mon, 05 May 2008 10:25:06 +0200, Matthias Watermann <lists (AT) mwat (DOT) de>
wrote:

Many thanks Matthias - quite a bit to get my head round but will start
trying now!

Cheers

Geoff