What's wrong with this line in PHP?

Question

Code

$stid = oci_parse($conn, "SELECT c1 FROM t1 WHERE c2 = " . $_POST['username'] . " AND c3 = " . $_POST['password'] . "");

Problem

I get ORA-00904 and ORA-24374:

ORA-00904: string: invalid identifier
ORA-24374: define not done before fetch or execute and fetch

Replacing $_POST in the code with the fixed string that $_POST returns stops the error. It must be the $_POST or the way that I'm including it in the code.

For example, $_POST['username'] is Doe, using $_POST['username'] doesn't work but using Doe does.

DO NOT USE THIS CODE! [SQL Injection](http://en.wikipedia.org/wiki/SQL_injection) What you think would happen if i would put `1; drop table t1;` into password field? — Milan Halada, May 12 '14 at 12:16
I bet that [Tom](https://asktom.oracle.com/pls/apex/f?p=100:1:0) knows all Oracle numeric error codes by heart but mortals possibly don't. Please don't mutilate error codes when posting to a forum. — Álvaro González, May 12 '14 at 12:19
@Uriel_SVK As I said below to one of the, so-called 'answers', you're assuming too much. — user2656114, May 12 '14 at 12:27
@user2656114 well if you would be using parametrized queries you would have no problems with quotes.... — Milan Halada, May 12 '14 at 12:37

score 2 · Accepted Answer · answered May 12 '14 at 12:12

2

try to make quote query vars. looks like you are passing strings to db

$stid = oci_parse($conn, "SELECT c1 FROM t1 WHERE c2 = '" . $_POST['username'] . "' AND c3 = '" . $_POST['password'] . "'");

Also escape your post data to prevent sql injection.

answered May 12 '14 at 12:12

Rakesh Sharma

13,680
5
37
44

2

Escaping is the stone age mechanism. The OCI extension does not even provide the required function. There's just no reason to write insecure code on purpose. – Álvaro González May 13 '14 at 07:16

Álvaro González · Answer 2 · 2014-05-13T09:12:07.337

The main problem with your line of code is that it's an open door for SQL injection; a call to get your server hacked. Right in the manual page for the oci_parse() function you're already using you have a example on how to pass parameters to queries:

$stid = oci_parse($conn, 'begin myproc(:p1, :p2); end;');
oci_bind_by_name($stid, ':p1', $p1);
oci_bind_by_name($stid, ':p2', $p2, 40);

oci_execute($stid);

Your code could look like this:

$stid = oci_parse($conn, "SELECT c1 FROM t1 WHERE c2 = :username AND c3 = :password");
oci_bind_by_name($stid, 'username', filter_input(INPUT_POST, 'username');
oci_bind_by_name($stid, 'password', filter_input(INPUT_POST, 'password');
oci_execute($stid);

... though it's still be a good idea to do error checking on the return values. All three functions above return FALSE on error and you have oci_error() to fetch an array with last error message.

What's wrong with this line in PHP?

Code

Problem

2 Answers2