Keskustelu: Nettisivujen teko: PHP/MySQL-lauseen syntaksin oikeellisuus

Pitäisikö tämän lauseen toimia? Itse en sitä jostain syystä saa toimimaan, mutta ei myöskään syntax erroria heitä tai muutakaan erroria:

mysql_query("UPDATE asiakastiedot (Etunimi, Sukunimi, Email, Tunnus, Salasana, Osoite, Postinumero, Toimipaikka)
VALUES('{$etunimi}', '{$sukunimi}', '{$email}', '{$user}', '{$hashpass}', '{$osoite}', '{$postinumero}', '{$toimipaikka}' WHERE Tunnus = ('$sess_user') AND istuntoID =
('$sess_id')  LIMIT 1");}

SQL:ää osaamattomanakin uskallan väittää, että VALUES-listan lopusta puuttuu sulku.

Metabolix mainitsikin jo varsinaisen virheen, mutta tällainen simppeli debuggaus kannattaa pistää korvan taakse:

mysql_query("INSERT...") or exit(mysql_error());

Ja ei sitten lopulliseen versioon tuollaisia, käyttäjälle ei välttämättä ole hyvä kertoa tietokantavirheitä...

Mä oon tainu rupee tyhmäks... Noin yksinkertanen virhe! Mut mut eipäs toimi vieläkään! Ton ideana on mun palvelussa olla sellases paikaa, joka päivittää noi tiedot tietokantaan.

Itse suosin kyselyn rivittämistä, jolloin virhetilanteissa virhe on helppo hahmottaa.

mysql_query("
UPDATE
	asiakastiedot
SET
	Etunimi = '{$etunimi}',
	Sukunimi = '{$sukunimi}',
	Email = '{$email}',
	Tunnus = '{$user}',
	Salasana = '{$hashpass}',
	Osoite = '{$osoite}',
	Postinumero = '{$postinumero}',
	Toimipaikka = '{$toimipaikka}'
WHERE
	Tunnus = '{$sess_user}'
	AND istuntoID = '{$sess_id}'
LIMIT 1
") or exit(mysql_error());

Onko tuo istuntoID merkkijono? Ja katso vaikka mitä mysql_affected_rows() sanoo.

Juu on se merkkijono... Kun siis tää juttu tulee mun nettikaupan asiakastietojen muuttumiseen, niin ton koodin avulla olis tarkotus päivittää syötetyt tiedot. Ja toi istuntoID on sitten tollanen "yhdiste" jolla tsekataan, että käyttäjä on kirjautuneena laitoin sen vaan tohon WHERE -kohtaan, että se tarkentaisi sitä..

UPDATE-lauseen WHERE-kohdassa voi käyttää sellaisia kenttiä, mitkä löytyvät päivitettävästä taulusta. Siis jos asiakastiedot-taulusta ei löydy istuntoID-kenttää, niin tuo ei toimine.

Btw, näkemättä sen enmmän koodia, niin tuossa vaikuttaa olevan ihan kiva tietoturva-aukko... Ajatteleppa seuraava tilanne:

Teen oman sivun, jossa on formilla nuo samat kentät kuin sinun profiilinmuokkaussivulla - tosin kentissä on minun itseasettamani arvot ja oma formini lähtee automaattisesti sivunlatauksen jälkeen javascriptin voimalla tuonne sinun profiilinmuokkaussivulle. Sitten ujutan kauppasi käyttäjän lataamaan tuon oman sivuni silloin, kun hän on kirjautuneena.

Itse ongelmaan:

- Tee muuttuja $sql:
- kokoa tuohon muuttujaan tuo kysely (siirapin vinkki kyselyn jäsentämisestä helpottaa debuggausta)
- ajat kysely, kuten aikaisemmin mainitsin paitsi lisäät kyselyn tulostuksen virhetilanteessa: mysql_query($sql) or exit(mysql_error().'<br />'.$sql);
- tai sitten ihan vaan tulostat kysely ja katso mitä se sisältää

ajv kirjoitti:

Btw, näkemättä sen enmmän koodia, niin tuossa vaikuttaa olevan ihan kiva tietoturva-aukko...

Voisitkin saman tien kertoa tietämättömille, kuinka tuo hoidettaisiin järkevästi.

Eli siis kun muokataan kriittisiä tietoja, niin aina pyydetään se salasana ja tarkistetaan sen avulla, että käyttäjällä on oikeasti oikeus muokata ko. tietoja.

Sekä myös seuraava:
http://en.wikipedia.org/wiki/SQL_injection

ajv kirjoitti:

Eli siis kun muokataan kriittisiä tietoja, niin aina pyydetään se salasana ja tarkistetaan sen avulla, että käyttäjällä on oikeasti oikeus muokata ko. tietoja.

Näin asia on siis ollut kokoaika, että pyydetään käyttäjältä salasanaa ennen kuin tiedot annetaan päivittää... Mutta onko tässä tilanteessa mahdollista injektoitua sisään?

Kokeile syöttämällä etunimikenttään esimerkiksi seuraava teksti:

Keimo' WHERE 1 = 1; DELETE FROM asiakastiedot; UPDATE asiakastiedot SET etunimi = 'Keimo

HUOM! Jos tämä toimii, poistaa se kaikki rivit asiakastiedot-taulusta!

Entä mihin sinä tarvitset tuota LIMIT 1 ?
Eihän siellä varmaan ole useampia samanlaisia tunnuksia jne...

Pekka Mansikka kirjoitti:

Entä mihin sinä tarvitset tuota LIMIT 1 ?
Eihän siellä varmaan ole useampia samanlaisia tunnuksia jne...

Optimointia, tuolloin tietokanta ymmärtää että muutos halutaan vain yhteen, eikä yritä etsiä turhaan muita jotka täyttävät ehdon.

ajv kirjoitti:

Itse ongelmaan:

- Tee muuttuja $sql:
- kokoa tuohon muuttujaan tuo kysely (siirapin vinkki kyselyn jäsentämisestä helpottaa debuggausta)
- ajat kysely, kuten aikaisemmin mainitsin paitsi lisäät kyselyn tulostuksen virhetilanteessa: mysql_query($sql) or exit(mysql_error().'
'.$sql);
- tai sitten ihan vaan tulostat kysely ja katso mitä se sisältää

Fiksumpaa minusta on käyttää jotain erillistä kirjastoa tahi pelkkää omaa funktiota SQL-lauseiden ajamiseen, missä itse voi sitten itse määritellä kuinka virheisiin suhtaudutaan. Täten ei tarvitse kirjoittaa sataa kertaa samaa rimpsua
mysql_query($sql) or exit(mysql_error().'<br />'.$sql);
Vaan voi halutessaan ohjata virheet vaikka myöhemmin johonkin SQL-tauluun logitettavaksi, ja tarvitsee vain muuttaa sitä yhtä funktiota tai määritellä se kirjastoon tehtäväksi toimenpiteeksi.

Itse tein tuollaisen yksinkertaisen rajapintaluokan tietokannoille PHP5:lle ja MySQL:lle sille toteutuksen, mutta sitä pitäisi vielä vähän viilata ennenkuin sen kehtaa julkaista. Tosin löytyyhän noita valmiitakin, ainakin Pearilla oli jokin, ja eikös PHP5:ssäkin ole jokin fiksumpi tapa hoitaa nuo tietokanta-asioinnit?

@JTS:

Juu, jos taidot riittää, niin ehdottomasti noin. Mutta jos taidot eivät riitä debuggaamaan yhtä virheellistä SQL-lausetta, niin eipä kannata välttämättä itse lähteä omaa tietokanta-abstraktiota rakentamaan. ;)

Olen kokeillut sekä PEARin DB2:sta, että PHP5:sen PDO:ta. Aina olen kuitenkin palannut omien tietokantaluokkien käyttöön. Ne logittavat virheet nätisti tekstitiedostoihin, ja kehitysvaiheessa ne tarjoavat muitakin kivoja työkaluja pullonkaulojen selvittämiseen.

No joo, meni ehkä vähän turhan pitkälle...

Eli tehdä esim: tälläinen funkkari

<?php
function myMysqlQuery($query) {
  mysql_query($query) or exit("Mysql query contains errors:<br />".mysql_error());
}
?>

Ja käyttää tuota sitten niiden kyselyiden ajoon. Sitten kun on saanut koodit valmiiksi ja julkaisee sen yleisölle niin muuttaa tuon funktion virheilmoitusta, mysql-virheitä ei kannata loppukäyttäjälle asti näyttää sillä ne heikentävät tietoturvaa merkittävästi.

Exit/die sijaan voisi käyttää trigger_erroria, tai nykyaikaan ehkä throw new MySQLException. Sitten omalla exceptionhandlerilla kontrolloi sitä, mitä virheviestillä tehdään - tulostetaanko vai logitetaanko.