problem with executing my interactive comments/feedback form

[waarbs]

I think this is one of those simple oversight type problems. At this stage I am simply trying to record the submitted comments into my database - I have not looked at retrieving this info back to a web page yet. I am getting the following error with the asp which deals with my feedback form:



Microsoft JET Database Engine error '80040e14'

Syntax error in INSERT INTO statement.

/onlineMScDynamic/HCT/unit 1/session 2/Lo2/u1-s2-lo2.asp, line 187



The ASP code on the rest of the page looks like this:


<a name="comment"></a>
<%
'start asp code

Set ObjDbConnection = Server.CreateObject("ADODB.Connection")
'i want to set up a database connection

ObjDbConnection.ConnectionString = "Provider=Microsoft.Jet.OLEDB.4.0;Data Source=" & Server.MapPath ("/onlineMScDynamic/dB/HCT_activityFB.mdb") & ";"
ObjDbConnection.Open


SQLQuery = "INSERT INTO HCT_1-2-2 (studentName, comments) VALUES "
SQLQuery = SQLQuery & "('"
SQLQuery = SQLQuery & Request.QueryString("studentName") & "','"
SQLQuery = SQLQuery & Request.QueryString("comments") & "')"


'response.Write(SQLQuery("studentName"))
'response.Write(SQLQuery("comments"))
ObjDbConnection.Execute(SQLQuery)

'IF ObjDbConnection.Execute(SQLQuery) = true THEN
'response.Write("submitted")
'ELSE
'response.Write("failed")
'END IF
%>


Can anyone spot the problem?

The form can be found at:

http://unthank.nottingham.ac.uk/onlineMScDynamic/HCT/unit%201/session%202/lo2/u1-s2-lo2.htm

Thanks

[Spudhead]

Couple of things:

1. You use Request.Querystring, but your form submits via POST. To get values out of a HTTP POST, use Request.Form.

2. If it's a SQL problem, post the SQL. Uncomment the response.write(), do a test submission and post the SQL: let us see exactly what you're trying to run against the database.

3. I know you've got it commented out, but the 'Execute()' method of a connection object doesn't return True or False. It returns an open or closed recordset object.

4. The way you're assembling your SQL statement is leaving you wide open to a SQL injection attack: there's nothing to stop people putting malicious code in that could, at the very least, let them do whatever they wanted to your database.

5. Actually - you'll probably want to remove the path to your database in your post. And put your database somewhere up above your web root if you can. The way it is at the moment, I can just download the whole database and see everyones comments.

[waarbs]

Thanks for the help and advice Spud.

About point number 4.)

"4. The way you're assembling your SQL statement is leaving you wide open to a SQL injection attack: there's nothing to stop people putting malicious code in that could, at the very least, let them do whatever they wanted to your database."

- where can I find more information to write more secure code and improve the security of what I am trying to do?


Cheers,
Mark

[Spudhead]

Google it. (http://www.google.co.uk/search?hl=en&q=ASP+SQL+injection+attack)

:thumbsup:

[waarbs]

Thanks again.


ANd about point 5.) - I can't work out how to write the path properly so that it points to the database above my wwwroot level - I keep getting syntax errors!

do I need to use something like ../dbExample/myDb.mdb ?

[Spudhead]

Well.. you can't use mappath() to get at directories outside your websites root directory. So the best you can do with it is to make sure you can always get a valid path to your web root, regardless of whether or not somebody moves your site into a different directory on the server.

So:

server.mappath("\")

That'll give you, for example, "c:\inetpub\wwwroot"

But you want to keep your database in a directory outside wwwroot. Because files in wwwroot might be accessible to internet weirdos. So - make a new directory in \inetpub\ and call it "secretHiddenStuff". Then you can do:

replace(server.mappath("\"), "wwwroot", "secretHiddenStuff")

You have to assume that those hardcoded directory names won't change, which they might. But it's generally pretty unlikely.

So your connection string would now look something like:

ObjDbConnection.ConnectionString = "Provider=Microsoft.Jet.OLEDB.4.0;Data Source=" & replace(server.mappath("\"), "wwwroot", "secretHiddenStuff") & "\myDatabase.mdb;"

Any help?

NB: This isn't the only solution, and it might not be the best one. But it will stop people downloading your database.

[waarbs]

Yes this is definately a help, cheers! I am spending time trying to put your comments together now and implementing them, but I still keep getting error messages relating to my sqlquery. I will write again to here when I have more specific info.

Mark