Keskustelu: Nettisivujen teko: Tietoturva sivuilani

Hei!
Olen tässä jo jonkin aikaa yrittänyt parannella sivustoni tietoturvaa. Ajattelin, että putkassa vois joku kokeilla krakkeroida sivujani, ilman että sekoittaa mitään. Ja aukon löytyessä kertoo siitä.

Linkki sivulle

Tai jos olisit järkevä, niin kertoisit suoraan miten olet sivut tehnyt, niin mahdolliset tietoturva-aukot löytyisivät kerralla.

öö tarkoitatko että heitän lähdekoodin tähän? :P

Sivun lähdekoodi ainakin on uskomattoman sekavaa. Jos PHP-puoli on vastaavaa sotkua, ei ole kovin epätodennäköistä, että siellä on myös muutama tietoturva-aukko.

Miksi kirjautumislaatikko muuten levenee, jos sen päälle vie hiiren?

Se on vaan sellanen pikku harjoitus, aivan turha tosin..

Lähdekoodin voisit tosiaan laittaa näkyville... Jos sitä on melko paljon, laita se vaikka palvelimelle jakoon, älä Putkaan.

Aukkojen etsiminen kokeilemalla on ihan hölmöä: mahdollisia temppuja on vaikka millä mitalla, ja niiden kaikkien kokeileminen on tolkuttoman työlästä. Jos haluat, että niitä oikeasti etsitään, laita koodi esille. (Muista piilottaa lähdekoodista salasanasi. Tai eihän sen koodin seassa kuuluisi ollakaan vaan jossain sivummalla asetustiedostossa.)

eli laitan vaikka txt tiedostoihin ne koodit palvelimelle ja linkkaan tänne? :)

Suosittelen dynaamisempaa ratkaisua:

<?php
$f =& $_GET["tiedosto"];
$sallitut = array("listaa.php", "tahan.php", "sallitut.php");
if (in_array($f, $sallitut)) {
  highlight_file($f);
} else {
  echo "<ul>";
  foreach ($sallitut as $f) {
    $url = urlencode($f);
    $html = htmlspecialchars($f);
    echo "<li><a href='?tiedosto={$url}'>{$html}</a></li>";
  }
  echo "</ul>";
}

(Edit: tiedostolistaus...)

http://ekuvat.dy.fi/lahdekoodit.php?tiedosto­=haku.php

Tosta noi ja sallitut tiedostot on:
haku.php
hsanat.php
index.php
kuva.php
laajennettu.php
lisaakuva.php
ylaosa.php

kuva.php:ssa on ainakin unohtunut syötteen filteröinti.

Kommenttien laatuun voisit hieman panostaa. "// Oman funktion is_banned luominen" ei ole hyvä kommentti. Kerro mielummin mitä is_banned() tekee.

haku.php & hsanat.php
- Regexeissä ei ole mitään järkeä, ota ne pois.
(Sama kaikkiin muihin skripteihin, missä noin on tehty.)

laajennettu.php:
- Et tarkista syötteitä vaan laitat ne suoraan SQL-kyselyyn -> fail.

lisaakuva.php:
- Copy-paste-koodisi on täyttä paskaa, syötteitä ei tarkisteta ikinä.

ylaosa.php:
- TÄYDELLINEN FAIL kieltää salasanasta muut kuin kirjaimet ja numerot!
- Salasanaa ei muutenkaan pidä mankeloida millään tavoin vaan käytetään as-is.
- Et ole suolannut tietokantaan tallennettuja tiivisteitä vaan käytät ainoastaan turvatonta md5-hashia pelkästä salasanasta.

Kaikki tietokantaan kirjoitettava data tulee varmistaa, mutta niin tulee varmistaa myös kaikki tietokannasta luettava data, joka laitetaan HTML-dokumenttiin. Tämä hoituu esim. funktiolla htmlspecialchars().

Älä copy-pastea toisten koodia, josset aio käydä sitä läpi ja ymmärtää sitä ja ottaa opiksesi siitä. Tuolta paistaa läpi kaikki ne kohdat, jotka olet vain copy-pastennut toisilta ja liimannut omaan koodiisii. Omaa koodiasi et ole ollenkaan vaivautunut siistimään esimerkkien antamien neuvojen mukaisesti. Toisaalta olet myös kopioinut vaarallista koodia, joka sinun pitäisi jo nyt tietää sellaiseksi, etkä ole korjannut sitä ollenkaan.

Olen melkein loukkaantunut siitä, että olet vain leikannut ja liimannut antamani koodinpätkän 1:1 omaan koodiisi, etkä ole vaivautunut poistamaan turhiksi jääneitä osia omastasi, vaikka korvaava koodi on laadultaan miljoonasti parempi. Nyt siellä on samat asiat kahteen kertaan: oikein ja väärin. EI NÄIN!

Tuntuu jokseenkin turhalta korjata nykyisen koodisi aukkoja, kun tietää että tulet tekemään samat virheet uuden koodisi kanssa tulevaisuudessa.

The Alchemist kirjoitti:

haku.php & hsanat.php
- Regexeissä ei ole mitään järkeä, ota ne pois.
(Sama kaikkiin muihin skripteihin, missä noin on tehty.)

Millä perusteella niistä ei ole mitään hyötyä?

The Alchemist kirjoitti:

mutta niin tulee varmistaa myös kaikki tietokannasta luettava data, joka laitetaan HTML-dokumenttiin.

Tätäpä en ole tiennyt, että tietokannasta tuleva tieto voi olla vahingollista, vaikka sinne ei välttämättä ole mahdollista tunkea sitä vahingollista tietoa, mutta korjataampa nekin. Tiedän, että jos korjaan rexepeillä, niin tulee sanomista, joten kokelempa tota htmlspecialchars() -funktiota.

dartvaneri muistuttaa erehdyttävästi paratiisin poikaa tyyliltään.

Mitä tarkoitat tolla noutti?

Ja erästä toista tattisivustoa pitävää.

1) Heität samaa tietoturvasettiä
2) Saat vastauksia, joista suutut
3) Copypastaat koodit muuttamatta mitään.

noutti kirjoitti:

Ja erästä toista tattisivustoa pitävää.

1) Heität samaa tietoturvasettiä
2) Saat vastauksia, joista suutut
3) Copypastaat koodit muuttamatta mitään.

Mutta en voi sille mitään, jos putkan oppaiden ja koodivinkkien perusteella tietoturva jutut on tehty. Mutta mitään yhteyttä ei näihin sivuihin eikä tyyppeihin ole..

Oonko suuttunut jostain kommentista? :P
Jos tätä meinaat, niin hain tarkennuksta The Alchemistin vastaukseen:

dartvaneri kirjoitti:

Millä perusteella niistä ei ole mitään hyötyä?

Voiko pdo:ssa tarkistaa montako rivä lisättiin/muokattiin/poistettiin kyselyssä?

Millä pdo:ssa voi korvata mysqln:

mysql_fetch_assoc -funktio

dartvaneri kirjoitti:

The Alchemist kirjoitti:

haku.php & hsanat.php
- Regexeissä ei ole mitään järkeä, ota ne pois.
(Sama kaikkiin muihin skripteihin, missä noin on tehty.)

Millä perusteella niistä ei ole mitään hyötyä?

Syötteet varmistetaan käyttämällä PDO:ta ja prepared statementseja tai poikkeustilanteissa mysql_real_escape_string-funktiolla. Jos sinulla on jokin syy blokata tietyt merkit, esimerkiksi nimimerkkien pitämiseksi siisteinä, niin silloin voi käyttää regexejä ja muita kommervenkkeja. Räpeltämällä vain ammut itseäsi jalkaan, kuten nytkin teit salasanojen kanssa.

lainaus:

The Alchemist kirjoitti:

mutta niin tulee varmistaa myös kaikki tietokannasta luettava data, joka laitetaan HTML-dokumenttiin.

Tätäpä en ole tiennyt, että tietokannasta tuleva tieto voi olla vahingollista, vaikka sinne ei välttämättä ole mahdollista tunkea sitä vahingollista tietoa --

Et sinä voi tietää, millä ehdoin data on tietokantaan kirjoitettu. Tietokantaan kirjoittaminen ja sieltä lukeminen ovat kaksi toisistaan riippumatonta prosessia. Jos sinulla olisikin nyt rajoitetumpi softa, joka kirjoittaa tietokantaan vain tiettyjä merkkejä, niin saatat tulevaisuudessa haluta modernisoida sitä ja tehdä siitä sallivamman. Tällöin olet kusessa, koska joudut käyttämään paljon työaikaa myös lukupuolen korjaamiseen ja testaamiseen, koska teit asiat väärin ensimmäisellä kerralla.

Poikkeuksen muodostavat ne softat, jotka muodostavat tulostettavan HTML:n jo kirjoitusvaiheessa. Tällöin tulosteita ei tietenkään voi enää ajaa htmlspecialcharsin läpi, koska muotoilut hajoavat.

PDO:n käyttämisessä voit turvautua manuaaliin: PHP: PDO.

Kannattaako mun kuitenkin htmlspecialchars() -funktion läpi vetää kaikki ennen, kuin laitan sen pdo kyselyyn? Vai onko sekin turhaa?

dartvaneri kirjoitti:

Kannattaako mun kuitenkin htmlspecialchars() -funktion läpi vetää kaikki ennen, kuin laitan sen pdo kyselyyn? Vai onko sekin turhaa?

Ei siinä ole mitään järkeä. Muotoilut pitäisi tehdä tulostuksen yhteydessä; tällöin niitä voi helposti muuttaa. Jos ajat esim. foorumeiden BBC-koodimuotoilut jo kirjoituksen yhteydessä, olet jumissa niiden kanssa koko loppu elämäsi. Jos taas muotoilut suoritetaan vasta tulostaessa, voit muutella HTML-koodia helpostikin versiosta toiseen.

Pakollisena esimerkkinä poikkeuksesta joku miljoonan käyttäjän boardi voisi perustella muotoiluiden kovakoodaamista sillä, että prosessointi tarvitsee silloin suorittaa vain kerran, mikä säästää kenties laskentatehossa.

Eli, jos ymmärsin asian oikein, niin tämä riittää:

<?php
	if($user){
		include('yhteys.php');
		$hakusana = $_POST['hakusana'];

		echo "Hakusana: $hakusana<br><br>";

		$kysely = $yhteys->prepare("SELECT * FROM hakusanat WHERE hs1 = ? OR hs2 = ? OR hs3 = ?");
		$kysely->execute(array($hakusana, $hakusana, $hakusana));

		while($tulos = $kysely->fetch()){

			$kuva_kannasta = $tulos['kuva'];

			$kuva_tark = htmlspecialchars($kuva_kannasta);

			echo "<a href='index.php?sivu=kuva&kuva=$kuva_tark'><img src=\"./kuva/$kuva_tark\" width=\"150\"></a>";

		}
	}
	else{
		echo "Ei oikeuksia!";
	}
?>

Onko tämä miten kattava salasanan suojaus?

        $salasana = "omena";
        $salasana_sha = sha1($salasana);
	$suola = "2m8mr3o7ak9Ka5mJ030ka2ms8";
	$suola_2 = "z88m2mak96ol10sn5mal";
	$suola_sha = sha1($suola);
	$suola_sha_2 = sha1($suola_2);

   $ssana_sha_suola = sha1($salasana_sha.$suola_sha.$suola_sha_2);

Ei useammalla suolalla tai hashauksella tee mitään.

sha1( $salasana . $suola )

Okei :)

Osaisko joku sanoa, että miksi tämä koodi heittää sivun valkoiseksi?

http://ekuvat.dy.fi/lahdekoodit.php?tiedosto­=ylaosa.php

ylaosa.php:n kuuluukin tuottaa tyhjä sivu, mutta jos tarkoitit indeksiä, niin hyvinhän sieltä tuli sisältöä.

Joo kokeilin kirjautua ulos, sitten toimii, mutta kun kirjaudun sisään, se menee valkoiseksi,se kirjautuu ilmeisesti, koska pitää kirjautua ulos, jotta näkee sivun sisällön. Oisko vika sessioneissa?

Edit. Ei vika voi sessioissa olla (ellei sessiot sitten tuhoudu ennen aikojaan), koska kokeilin laittaa tuossa käyttäjän tunnustus kohdassa

if(isset($_SESSION['log_key'])){

koodin jälkeen

echo "$_SESSION['log_key']";

ja se tulosti sen log_keyn arvon.

Osaako joku sanoa, mikä on vikana tuossa, että se ei kirjaudu
Edit. Voisko olla vikana tuo taulukko?

$user = array($tunnus_tark => 'tunnus', $salasana_tark => 'salasana', $id_tark => 'id');

		//lisätään vielä käyttäjän viimeisestä sivunlatauksesta aikaleima
		$kysely = $yhteys->prepare("UPDATE {$tbl_users} SET last_load = NOW() WHERE id = ".$user['id']);
		$kysely->execute();

Ku en tiedä löytääkö toi kysely tota:

$user['id']

Taulukossa on avain ja arvo väärin päin.

<?php
$user = array('tunnus' => $tunnus_tark); //lisää loput

Ja kun kerran käytät PDO:ta, miksi laitat parametrit suoraan SQL-kyselyyn?
https://www.ohjelmointiputka.net/oppaat/opas.php?tunnus=mysqlphp02#kyselynparametrit

kiitos! :)
Jos tätä tarkoitat:

$kysely = $yhteys->prepare("UPDATE {$tbl_users} SET last_load = NOW() WHERE id = ".$user['id']);
$kysely->execute();

niin se näköjään on unohtunut, jota ei kyllä saisi tapahtua.

Edit. Nyt se toimii.
Mitenkä muuten ton kirjautumis sydeemin tietoturvan laita on? Aukkoja?

Muuten hoksasin tällaisen jutun The Alchemistin kommentissa:

The Alchemist kirjoitti:

haku.php & hsanat.php
- Regexeissä ei ole mitään järkeä, ota ne pois.
(Sama kaikkiin muihin skripteihin, missä noin on tehty.)

laajennettu.php:
- Et tarkista syötteitä vaan laitat ne suoraan SQL-kyselyyn -> fail.

Ja siitä tuli mieleen pieni kysymys, miksi esim haku.php:ssä ei tarvita regexejä, tai yleensäkkään mitään tarkistusta, mutta laajennettu.php:ssä tarvii, siis ennen sql-lauseeseen sijoittamista? Vai tarkoitatko tuossa laajennettu.php:ssä noita OR ja AND syötteitä?

Nyt olis vähän päivitelty uploadiakin. Onko siinä jotain aukkoja vielä?

http://ekuvat.dy.fi/lahdekoodit.php?tiedosto­=lisaakuva.php

Osaisko joku sanoa, että miksi tämä ei anna lähettää isompia kuvia, kooltaan jotain 5-9 Mt? Se ei heitä virheilmoituksiakaan. Aikasemmin se heitti "Tiedoston koko on 0 - sitä ei voida lähettää".

dartvaneri kirjoitti:

Nyt olis vähän päivitelty uploadiakin. Onko siinä jotain aukkoja vielä?

http://ekuvat.dy.fi/lahdekoodit.php?tiedosto­=lisaakuva.php

Osaisko joku sanoa, että miksi tämä ei anna lähettää isompia kuvia, kooltaan jotain 5-9 Mt? Se ei heitä virheilmoituksiakaan. Aikasemmin se heitti "Tiedoston koko on 0 - sitä ei voida lähettää".

php.inisssä upload_max_filesize, post_max_size, memory_limit rajoittaa uploadtiedoston koon. näitä kasvattamalla voit nostaa maksitiedosto kokoa.

Muuten hoksasin tällaisen jutun The Alchemistin kommentissa:

The Alchemist kirjoitti:

Ja siitä tuli mieleen pieni kysymys, miksi esim haku.php:ssä ei tarvita regexejä, tai yleensäkkään mitään tarkistusta, mutta laajennettu.php:ssä tarvii, siis ennen sql-lauseeseen sijoittamista? Vai tarkoitatko tuossa laajennettu.php:ssä noita OR ja AND syötteitä?

Muuttujien $valinta1 ja $valinta2 käyttö suoraan SQL-lausekkeessa altistaa injektiolle, joten niiden sisältö täytyy varmistaa mysql_real_escape_string():llä ensin.

Ja jos ollaan tarkkoja, niin on väärin toteuttaa tuo kyselyn manipulointi siten, että KÄYTTÄJÄN täytyy syöttää käytettävä ehto (and/or). Parempi olisi tehdä niin, että tarkistat onko checkbox ruksittu - tai mitä sitten käytätkään - ja valitset ehdon sen mukaan.

HTML:
<input type="checkbox" name="use_and_clause"/>

PHP:
$valinta = $_GET['use_and_clause'] == 'on' ? 'AND' : 'OR';

En tiennytkään että

<select name="">
<option value="jotain">jotain
</select>

tohon selectiin voidaan kirjoittaa, joten taidanpa korjata mahdollisimman pian :)

Selectiin ei voi kirjoittaa suoraan, mutta käyttäjä voi tehdä oman sivun, jossa on erilainen lomake tai lähettää injektiota palvelimelle esim. cURL:illa tai Netcatilla.

täytyy korjata, vaikka voi olla hieman ongelmallista saada sitä lomaketta lähetettyä, ku on toi turvatarkistus tossa lomakkeessa.

Edit. Nyt on korjattu.
http://ekuvat.dy.fi/lahdekoodit.php?tiedosto­=laajennettu.php

Siis hä? Mikä tuosta nyt tekee yhtään vaikeamman väärentää? Ota vain pois tuo ylimääräinen piilokenttä ('tark'), sillä ei ole minkäänlaista funktiota tuossa lomakkeessa. Se ainoastaan kuluttaa palvelimen kaistaa, kun pitää lähetellä turhaa dataa verkon yli.

Sellainen käytännön ohje vielä, että lomakkeen tiedot kannattaisi käsitellä aina ns. välisivulla, ainakin kun käytetään post-metodia. Datan käsittely samalla sivulla aiheuttaa joissain selaimissa ärsyttävän "haluatko varmasti lähettää tiedot uudestaan" -kyselyn. Lisäksi sivun ylläpito on sitä selkeämpää, mitä vähemmän PHP:tä HTML:n välissä on => kannattaa laittaa prosessointi tiedoston alkuun ja HTML tulostella vasta lopuksi ja kerralla.

if($valinta1 == 'Ja'){
  $hehto1 = 'AND';
} elseif($valinta1 == 'Tai'){
  $hehto1 = 'OR';
} else {
  exit('Virheellinen hakukysely. "{$valinta1}" ei ole validi.');
}

Tarpeettoman monimutkaista. Ei sinua kiinnosta kuin saada SQL-kysely kasaan.

Siis: $valinta1 = $_GET['valinta1'] == 'and' ? 'AND' : 'OR';

No mietitäänpä:

Jos on sivu.com:ssa seuraavanlainen lomake:

<form action="index.php?sivu=haku" method="post">
    <input type="text" name="hs1">
    <input type="text" name="hs2">
    <input type="text" name="hs3">
    <input type="hidden" name="tark" value="2xlso1ka-asje">
    <input type="submit" value="Hae" class="hae">
</form>

Ja sitten php osuus:

$hs1 = $_POST['hs1'];
$hs2 = $_POST['hs2'];
$hs3 = $_POST['hs3'];
$tarkistus = $_POST['tark'];
$tsana = "2xlso1ka-asje";

if($tarkistus == $tsana){
    //käsittely
}

Näin, eli jos se postilla tullut tarkistus koodi vastaa sitä koodia mikä on ennalta määrätty, niin jos esim. sivu.org:iin tehtäisiin lomake:

<form action="http://sivu.com" method="post">
    <input type="text" name="hs1">
    <input type="text" name="hs2">
    <input type="text" name="hs3">
    <input type="submit" value="Hae" class="hae">
</form>

Siis kun se on hidden se yksi kenttä, niin sitä ei näe ja täten sitä ei löydy sivu.orgin lomakkeesta ja näin ollen se ei anna lähettää tietoja.

Korjatkaa jos olen väärässä.

The Alchemist kirjoitti:

Siis: $valinta1 = $_GET['valinta1'] == 'and' ? 'AND' : 'OR';

Voisiko joku selittää mulle tämän tominnan?
Ja siis mikä kohta on if-lauseessa samaa tekevä kohta. Olen käsittänyt että tämä vastaa if-else -lausetta. Jos näin on, voiko olla elseifiä? :)

dartvaneri kirjoitti:

Siis kun se on hidden se yksi kenttä, niin sitä ei näe ja täten sitä ei löydy sivu.orgin lomakkeesta ja näin ollen se ei anna lähettää tietoja.

Jaa, meinaat että joku joka osaa tehä lomakkeen, joka postaa sun skriptihandlerilles ei osaa kopioida sitä hidden-inputtia? Ei kuulosta kovin todennäköseltä skenaariolta.

En ole oikeen perillä krakkeroinnista, niin en tiennyt että se on mahdollista, mutta kiva tietää, ja jos tarkemmin ajattelee, niin eipä siitä pahemmin ole hyötyä.

Kyllähän se nyt kuuluu webbikoodauksen perusteisiin ymmärtää, että HTML-sivu ja sen lähdekoodi ovat suoraan nähtävissä itse selaimessa, tai kun HTML-dokumentin aukaisee vaikka tekstieditorilla.

P.S. Josset ymmärrä koodin toimintaa, niin testaa sitä ja leiki sen kanssa, kunnes alat ymmärtää.

elikkä..

$valinta1 = $_GET['valinta1'] == 'and' ? 'AND' : 'OR';

jos $_GET['valinta1'] on 'and' niin silloin $valinta1 on 'AND' ja jos ei ole niin $valinta1 on 'OR'.. ookkei :D Mutta tällähän ei voi korvata kaikkia if-lauseita?

Voisko joku sanoo jotain noiden ylaosa.php:n ja lisaakuva.php:n tietoturva-aukoista? Ja muutenkin sivun tt-aukoista..

Edit. lisäsin myös lataa.php:n tonne sallittujen listoille.

*** ylaosa.php

$kysely = $yhteys->prepare("SELECT * FROM {$tbl_users} WHERE istunto = ?");
$kysely->execute(array($_SESSION['log_key']));
$tulos = $kysely->fetch();

$id_tark = htmlspecialchars($tulos['id']);
$user = array('tunnus' => $tunnus_tark, 'salasana' => $salasana_tark, 'id' => $id_tark);

$kysely = $yhteys->prepare("UPDATE {$tbl_users} SET last_load = NOW() WHERE id = ?");
$kysely->execute(array($user['id']));

Sano nyt plz, että näet itsekin, miksi tuo koodi on rikki? (En tarkoita $tbl_users-muuttujaa, joka on toivottavasti määritelty jossain toisaalla.)

Otetaan vastaan valistuneita arvauksia.

Lataa.php:stä minulla on vain sen verran sanottavaa, että kun aiemmin valitin että kopioit vain koodia meiltä etkä yritä oppia siitä mitään, niin kyseinen filu osoittaa, että noudatat edelleen samaa linjaa.

Sellanen kysymys tässä ennen ku lähden arvaileen, että olet tarkoituksella ottanut poies tosta muuttujien $tunnus_tark ja $salasana_tark tarkistukset? Tästä se ei oikeestaan rikki ole, mutta toi on kai ihan turha toi id:n tarkistus, ku sehän määrittyy automaattisesti?

Ei se ole vain turha; periaatteessa se voisi rikkoa koko tuon jälkimmäisen kyselyn. Et sinä voi lukea tietokannasta tavaraa, muokata sitä, ja sitten käyttää sitä uudelleen matchaamisen ehtona. Tästä päästään siihen pointtiin, mitä olen hokenut luultavasti tässäkin ketjussa: pitää kyetä tunnistamaan vainoharhaisuus ja ottamaan järki käteen.

Htmlspecialchars() ei muuten tarkista yhtään mitään.

Eli hmm.. Jos nyt oikeen tulkitsin, niin se virhe on tässä:

$id_tark = htmlspecialchars($tulos['id']);

eli siis, tämä pitäisi olla poissa, jotta se ei muuta ton id:n erikoismerkkejä html muotoon vahingossakaan, koska sitä käytetäät jälkeenpäin hakukyselyssä?

Mitenkäs se id ton alemmankyselyn muuten voi rikkoa? Kun tänne on postattu jo monesti se, että toi execute estää ne sql-injektiot, ja toi id muuttuja menee sen kautta?

The Alchemist varmaankin tarkoitti rikkomisella nyt sitä kuvitteellista tilannetta, että id on vaikkapa "<>". Tästä tulisi htmlspecialchars-funktiolla "&lt;&gt;", minkä jälkeen jälkimmäinen kysely ei toimi enää, koska kannassa ei ole riviä, jonka id olisi "&lt;&gt;". Toki id on oikeasti numeerinen, mutta jos yhdessä kohti on tuollaista hölmöilyä, ei olisi mikään yllätys, jos olisit tehnyt saman tempun jossain tärkeässäkin kohdassa.

Eli älä arvaile niiden funktioiden käyttötarkoituksia vaan käytä niitä järkevästi. Jos asiasta on vähänkin epäselvyyttä, ota nyrkkisäännöksi vaikka sellainen, että htmlspecialchars esiintyy vain ja ainoastaan tiedon tulostuksen yhteydessä (echo, printf ym.) ja siellä esiintyykin sitten jokaisen muuttujan ympärillä. Osoitteet (<img src="osoite">, <a href="osoite"> ym.) puolestaan pitää ajaa urlencode-funktion läpi, tähän sopii sama nyrkkisääntö kuin edelliseen, ja luonnollisesti jompikumpi funktio riittää.

Muutenkin on intuitiivista, että käyttäjätiedot käsittävä tietue sisältää käyttäjän tiedot alkuperäisessä muodossaan. Ne eivät ole siellä HTML-dokumenttiin tulostamista varten vaan kulissien takana tapahtuvaan autentikointiin ja muihin toimintoihin. Viestissäni ei ollut tulkittavaa, sanoin asiat ihan suoraan.

Eli siis, otetaanpa esimerkki tuolta index.php:stä :

if(!$user){
        //kirjautuminen
}
else{

   				echo "<font color=\"black\" valign=\"middle\"> <b>".$user['tunnus']."</b>! |   </font><a href=\"index.php?logout\"><font color=\"black\" valign=\"middle\">Kirjaudu ulos  </font></a><br />";

			}

niin tämä pitäisi korvata seuraavalla koodilla:

if(!$user){
        //kirjautuminen
}
else{
                 $tunnus = htmlspecialchars($user['tunnus']);
   				echo "<font color=\"black\" valign=\"middle\"> <b>{$tunnus}</b>! |   </font><a href=\"index.php?logout\"><font color=\"black\" valign=\"middle\">Kirjaudu ulos  </font></a><br />";

			}

Ja sitten poistaa html-muotoon muuttaminen tuola taulukkoon sijoittamisen yhteydestä. Olenko oikeilla jäljillä? :)

Kaikki on selitetty edellisissä viesteissä, toistamisesta tuskin on erityistä apua.

Löytyykö tosta lisaakuva.php :stä vielä paljoltikin paikattavia aukkoja?

Teknkik: jos sisäänkirjautumisessa on sql-injektion vaara, niin korjauksen pitäisi olla hyvinkin yksinkertainen.

Olen näkevinäni kirjautumisessa ihan oikeat prepare- ja execute-rivit, jotka eivät jätä sijaa injektioille. Toki htmlspecialchars on ihan pelleilyä, mutta se ei ole tietoturvaongelma.

Hei! Osaisiko joku sanoa, miksi tämä koodi kirjautuu tyhjillä sivulle, siis hyväksyy tyhjät, ja sitten jos kirjautuu oikeilla tunnuksilla niin se kirjaa sen silti tyhjänä?

http://porha.dy.fi/lahdekoodit.php

ylaosa.php ja Kirjaudu.php

Kirjautumisessa olisi fiksua tarkistaa montako riviä kysely palautti. Mikäli ei saatu yhtään riviä ei kannata kirjoittaa sessiotakaan. htmlspeciachars funktiota käytetään edelleen väärin, joskaan se ei kaiketi ole kovin kamala virhe. Yleisesti sitä käytetään juuri ennen tulostusta.

Ok. Tarkoitat kai tätä kyselyä?:

$kysely = $yhteys->prepare("UPDATE {$tbl_users} SET istunto = ? WHERE tunnus = ? AND salasana = ?");
		$kysely->execute(array($istuntotunnus, $tunnus, $ssana_sha_suola));

Tietenkin tarkoitan tuota kyselyä, koska sen avullahan sinä kuvittelet tunnistavasi henkilöt eikös juu? Sinällään korjaushan on helpohko, joten keksit sen varmasti itsekin.

Elikkäs näin:

$kysely = $yhteys->prepare("SELECT COUNT(*) FROM kayttajat WHERE tunnus = ?");
$kysely->execute(array($tunnus));

$tulos = $kysely->fetch();
	$maara = $tulos["COUNT(*)"];
	if($maara != 0){

		$kysely = $yhteys->prepare("SELECT * FROM kayttajat WHERE tunnus = ?");
		$kysely->execute(array($tunnus));

		$tulos = $kysely->fetch();
		$salasana_kanta = $tulos['salasana'];
		if($salasana_kanta == $ssana_sha_suola){
			$istuntotunnus = md5(uniqid(""));
			$kysely = $yhteys->prepare("UPDATE {$tbl_users} SET istunto = ? WHERE tunnus = ? AND salasana = ?");
			$kysely->execute(array($istuntotunnus, $tunnus, $ssana_sha_suola));

			$_SESSION['log_key'] = $istuntotunnus;  //laitetaan sessioon talteen istuntotunnus

			header("Location: index.php?sivu=Etusivu");
		}
		else{
			$error .= 'Tunnus ja salasana eivät täsmää.<br>';
		}
	}
	else {

	$error .= 'Tunnusta ei löytynyt.<br>';
	}

Nyt se toimii. Kiitos! :) Onko tämä koodi nyt mitä ajat takaa?

No en minä nyt ihan tuotakaan ajatellut. Mutta jokin tällainen käväisi mielessä. Tosin koodivinkki on jo itsessään huomattavasti turvallisempi, kuin tuo sinun viritelmäsi. Tuohon voisit vain muokata PDO noiden raakojen mysql_* funktioiden tilalle.

<?php
$kysely = $yhteys->prepare("SELECT tunnus FROM kayttajat WHERE tunnus = ? AND salasana = ?");
$kysely->execute(array($tunnus, $salasana));

if ($kysely->rowCount() === 1) {
/** Tähän ne sinun koodit **/
} else {
/** Tyhjä tulosjoukko ilmoita virheestä **/
}
?>

Teuro kirjoitti:

Tuohon voisit vain muokata PDO noiden raakojen mysql_* funktioiden tilalle.

Jos oikein ymmärsin, juuri tätä olen yrittänyt, vaikka tosin ilmeisesti epäonnistunut siinä.

Ok, yritin googlettaa tota kyselyn palattamien rivien määrä, mutta en löytänyt sitä.

PDO:n rowCount ei ole SELECT-kyselyitä varten vaan UPDATE-kyselyitä varten. Aiempaan koodiin (kohdassa "tarkoitat kai tätä kyselyä") tarvitsee siis vain lisätä perään tarkistus:

if ($kysely->rowCount() == 0) {
  $error = "Tunnusta ei löydy!";
} else {
  // $_SESSION jne.
}

Toki tässä on se riski, että istuntotunnuksesta sattuu tulemaan uudestaan sama eikä rivi muutukaan. Siksi kannattaisi ottaa tuo md5-kutsu pois ja käyttää pelkkää uniqid:tä.

$istunto = uniqid("", true);

Kiitos vinkistä! :)

Edit.
Nyt se näyttää tältä:

					$istuntotunnus = uniqid("", true);
					$kysely = $yhteys->prepare("UPDATE {$tbl_users} SET istunto = ? WHERE tunnus = ? AND salasana = ?");
					$kysely->execute(array($istuntotunnus, $tunnus, $ssana_sha_suola));
						if ($kysely->rowCount() == 0) {
 							 $error = "Tunnusta ei löydy!";
						} else {
							$_SESSION['log_key'] = $istuntotunnus;  //laitetaan sessioon talteen istuntotunnus

							header("Location: index.php?sivu=Etusivu");
						}

Ja toimii hienosti