Keskustelut - PHP - Mysql tietojen päivittäminen


editoitu: 14:30 30.12.09
tmaki 14:14 30.12.09 
Minun pitäisi saada päivitettyä mysql kannassa olevat tiedot... ole käyttänyt tässä lomakkeena ja käsittelijänä täältä löytyvää mallia: https://wiki.hamk.fi/display/materiaalit/8.+MySQL+ja+PHP

Nyt ongelmana tämän mysql lauseen toteutus $sql="select * from hinta where id=" . $_GET["id"]; jolla pitäisi hakea lomakkeelle päivitettävät tiedot näkyviin... tämä toimii ihan ok jos laitan id:n numeerisessa muodossa, mutta ei tällä kyseisellä lauseella... kertokaapas viisaammat missä mennään metsään.

tarkoitus tehdä hinnasto, jota voidaan päivittää tarpeen mukaan...
raspi 14:19 30.12.09 
No hetkinen seis miten tietoturvatonta koodia. Hyvä esimerkki miten juttuja EI pidä tehdä.
weicco 14:43 30.12.09 
$sql="delete from tuotteet where id=" . $_GET["id"] . " limit 1";

Poista.php?id=1 or 1=1

Bingobangobongo!
tmaki 11:54 31.12.09 
ei sen vaan tämänkään mallin avulla hae tietoa kannasta lomakkeeseen jos sen voisi päivittää uuteen arvoon.
qin 12:02 31.12.09 
tmaki kirjoitti:
ei sen vaan tämänkään mallin avulla hae tietoa kannasta lomakkeeseen jos sen voisi päivittää uuteen arvoon.


Et tainnut ymmärtää weicon pointtia? Ei tarkoituksena ollut (eikä muutenkaan) antaa valmista koodia vaan vinkkiä siitä miten tuontyyppinen sql-lauseke ylipäätään on tietoturvaton.
weicco 12:55 31.12.09 
"Nämä sivut ovat tehty Hämeen Ammattikorkeakoulun tietojenkäsittelyn koulutusohjelman käyttöön PHP-perusteiden opiskeluun"

Note to self: Älä koskaan hae ainakaan Hämeen ammattikorkeakouluun...

Itse ongelmaan. En nyt ymmärrä mikä on ongelma. Toimii vain numeerisilla arvoilla? Niin kai sen pitäisi, kun ID on numeerinen arvo... Tarkista nyt ainakin kuitenkin, että $_GET['id'] on sellainen is_numeric funktiolla. Tai itseasiassa siirry johonkin fiksumpaan kuten PDO:on tai mieluusti kokonaan fiksumpaan ohjelmointikieleen.
timoh 14:50 31.12.09 
weicco kirjoitti:
Tarkista nyt ainakin kuitenkin, että $_GET['id'] on sellainen is_numeric funktiolla.

Sanompa sivuhuomautuksena että is_numeric ei ole hyvä funktio tällaiseen tilanteeseen, se kun hyväksyy kaikenlaisia numeerisia merkintöjä (+0123.45e6 jne.). ctype_digit on pastellimpi. Mutta totta tosiaan tuo Hamkin "mysql ja php" -materiaali on... Mistä näitä tietoturva-aukkoja oikein tulee! Tietysti perusteet perusteina, mutta joku roti oppilaitoksissakin pitäisi olla. Tästä tuli heti mieleen kun joskus aiemmin taisin jotain kysellä miten suomalaisissa oppilaitoksissa opetetaan tietoturva-aspekteja tjms. koodaukseen liittyen ja tässähän nyt sitten on oikein kunnon hot spot.
weicco 14:55 31.12.09 
Jajjoo. Olisi pitänyt lukea dokumenttia tarkemmin. Mutta mitä helvettiä! Sano, että käsitin tuon ensimmäisen kommentin tuolla ctype_digitissä väärin ja että tämä ei pidä paikkaansa "Be aware though, before PHP 5.1.0 this will return true on an empty string" :D

Tähdennän vielä edellisestä kommentistani olennaisen: itseasiassa siirry johonkin fiksumpaan .. mieluusti kokonaan fiksumpaan ohjelmointikieleen
timoh 16:24 31.12.09 
Changelogin mukaan kyseessä ei ole suinkaan edes bugi, joten kyse on selkeäst(ä)i ominaisuudesta ;)

T.M. 19:06 2.1.10 
is_numeric() vain tarkistaa numeron.. ei muokkaa sitä ollenkaan, homman voi hoitaa helposti näin:
PHP
$sql="select * from hinta where id=" . (int)$_GET["id"]; 
eis 23:31 3.1.10 
"vain", eikös tarkoituksena nimenomaan ollutkin tarkistaa eikä muokata. Muokkaamiselle ei pitäisi tuossa tilanteessa olla tarvetta.

Ja huoh, vilkaisimpa itsekin ctype_digitin manuaalisivua... eli tuollekin näköjään on parempi tehdä itse funkkari kuin luottaa valmiiseen..
editoitu: 01:40 4.1.10
tsuriga 01:30 4.1.10 
PHP:ssä on myös filtteröinti.


PHP
<?php

// 1. $_GET['id'] = '2'
// 2. $_GET['id'] = 'd25850'

// Koko GET-taulukon suodatus kerralla

$definition = array(
    'id' => array(
        'filter'  => FILTER_VALIDATE_INT,
        'options' => array('min_range' => 1)
    )
);

$input = filter_input_array(INPUT_GET, $definition);
var_dump($input['id']); // 1. int(2), 2. bool(false)

// Yhden INPUT-arvon suodatus

$options = array(
    'options' => array(
        'min_range' => 1
    )
);

$id = filter_input(INPUT_GET, 'id', FILTER_VALIDATE_INT, $options);
var_dump($id); // 1. int(2), 2. bool(false)

?>
eis 07:53 4.1.10 
Mjuu, hyvä pointti. PHP:n filttereissäkin on omat muistettavansa silti, esim FILTER_VALIDATE_INT ei hyväksy kuin numeroita (ei siis esim etuplussaa tai -miinusta), mutta FILTER_SANITIZE_NUMBER_INT sit taas hyväksyykin ne. Ja "Note: Numbers +0 and -0 are not valid integers but validate as floats. ". Puuh.
weicco 08:22 4.1.10 
+0.0f validoituu varmaan sitten integeriksi :D
editoitu: 19:36 12.1.10
T.M. 19:31 12.1.10 
T.M. kirjoitti:
is_numeric() vain tarkistaa numeron.. ei muokkaa sitä ollenkaan, homman voi hoitaa helposti näin:
PHP
$sql="select * from hinta where id=" . (int)$_GET["id"]; 
eis kirjoitti:
"vain", eikös tarkoituksena nimenomaan ollutkin tarkistaa eikä muokata. Muokkaamiselle ei pitäisi tuossa tilanteessa olla tarvetta.

en käsitä miksi tuota pitäisi tarkistaa sen koommin, kun niitä ei käsin sinne syötetä, vaan klikataan jostain linkistä, ja jos on väärin syötetty niin (int) kyllä oikaisee mutkat oikein kunnolla!

siis tässähän juuri käytettiin (int) jotta voitaisiin estää rikollinen toiminta jonka weicco heitti: "$sql="delete from tuotteet where id=" . $_GET["id"] . " limit 1";"

"Tarkista nyt ainakin kuitenkin, että $_GET['id'] on sellainen is_numeric funktiolla." minkä helvetin takia pitää tarkistaa, jos sen voi muokata samantien "oikeaksi" (int) käyttämällä ilman lisärivejä koodiin? jos joku nyt menee kirjoittaa vahingossa sinne "kakka" ja siitä tuleekin 0 PHP:n päässä, niin mitä sitten. voi voi.
eis 01:01 13.1.10 
Sen takia että ne on eri käyttötapaukset. Jos joku haluaa syöttää sinne pasketta, ei ole mitään syytä päästää sitä kantaan asti.
Grez 01:12 13.1.10 
No hei siis ideanahan on oikeinkin käytettynä, että käyttäjä saa poistaa vaikka kaikki rivit :D

Itse asiassa tässä yksittäisessä tapauksessa en keksi mitä hyötyä tuosta injektion estämisestä on.. mysql_query ei kuitenkaan anna ajaa useita kyselyitä samalla, joten Bobby Tables ei pitäisi onnistua.

Olette toki oikeassa, että parempi olisi vaan tehdä oikein joka kerta eikä miettiä tällaisia.
eis 12:42 13.1.10 
Grez kirjoitti:
Itse asiassa tässä yksittäisessä tapauksessa en keksi mitä hyötyä tuosta injektion estämisestä on.. mysql_query ei kuitenkaan anna ajaa useita kyselyitä samalla, joten Bobby Tables ei pitäisi onnistua.


Jos ois sallittua pistää mitä vaan arvoja, mites vaik

SQL
SELECT * FROM hinta WHERE id = id UNION SELECT user, password FROM mysql.user
 


Kaikki hostkonffit, tietokannannimet jne on samalla lailla saatavilla - kaikki mihin tietokannankäyttäjällä on oikeus.
Grez 12:47 13.1.10 
Niin no tarkoitin tuota weicon delete -esimerkkiä, joka ilman injektointiakin antaa poistaa ko. taulun kaikki rivit yksi kerrallaan:
weicco kirjoitti:
$sql="delete from tuotteet where id=" . $_GET["id"] . " limit 1";

Poista.php?id=1 or 1=1

Bingobangobongo!