Mscripting - CMS systeem

Ziet er leuk uit, maar dat Adminpaneel om staff te berheren.
Zit er wel iets bij als supervisor-rechten?

Nu kan iemand die Admin is, gewoon een andere Admin zijn rechten ontnemen? Lijkt me niet de bedoeling?

<?php
hmm();
?>

> Nu kan iemand die Admin is, gewoon een andere Admin zijn
> rechten ontnemen? Lijkt me niet de bedoeling?

Ik vrees dat een willekeurige gebruiker dat kan, zelfs. Er is geen rekening gehouden met beveiliging van het login formulier en ik vermoed dat dat niet de enige plek is. Helaas staat het niet online, maar ik denk dat ik minstens tien verschillende manieren weet om iets te doen met het systeem wat niet de bedoeling is.

In het geval van login; zie onderstaand.

<?php
/** Uit het formulier: */
$_POST['naam'] = 'admin\' OR 1=1; --';

$sql = "SELECT * FROM user WHERE naam='" . $_POST["naam"] ."' AND wachtwoord='". md5($_POST["wachtwoord"]) . "'"; 
$sqls = mysql_query($sql);
?>

En toen was ik admin, zonder wachtwoord.

-

> Ik vrees dat een willekeurige gebruiker dat kan, zelfs. Er is geen rekening gehouden met beveiliging van het login formulier en ik vermoed dat dat niet de enige plek is. Helaas staat het niet online, maar ik denk dat ik minstens tien verschillende manieren weet om iets te doen met het systeem wat niet de bedoeling is.

> In het geval van login; zie onderstaand. 

Uhm, ik wil hier niet veel tegen zeggen, maar dit is een eigen systeem wat je op je eigen site kan plaatsen. Als jij een account wilt zonder in te loggen, (zonder wachtwoord in te voeren), dan kan je dat daar door doen ja. Maar andere mensen, bezoekers, kunnen dat niet, die hebben daar niet de mogelijkheden tot. Dus als jij je systeem daar mee wil verpesten, dan moet je dat doen.

Het lijkt gewoon mogelijk te zijn. Er is totaal geen rekening dus gehouden met SQL-injection-beveiliging. Dus zonder wachtwoord kan je er al zo in komen.

Of je admin bent of niet.

<?php
hoi();
?>

Vermeld hier alle zaken betreffende valkuilen, handige informatie, (installatie-)instructies e.d.

Zomaar even op te noemen:

contact.php:

geen afhandeling voor mailcontrole.. 
geen headers mee gestuurd.. wat veelal in de spambox zal belanden...

Sql injectie... zie reactie Berry...

page.php

je maakt gebruik van de variabel: $rows
Je gebruikt hem nergens.. onnodig dus...


Overige feedback:
Script localhost gezet...
Nog geen database er achtergehangen...

Gelijk al 1 error die mij opviel:
Warning: mysql_fetch_array() expects parameter 1 to be resource, boolean given in C:\wamp\www\Mscripting-CMS\include\menu.php on line 8

Toen maar even install.php aangeroepen:
Notice: Undefined index: install in C:\wamp\www\Mscripting-CMS\install.php on line 94
type="text" name="naam" value="
Notice: Undefined index: naam in C:\wamp\www\Mscripting-CMS\install.php on line 94
">

ect ect ect...
Er mag dus nog genoeg gedaan worden

Ik wilde even reageren op de reactie van Marnix Peters. Blijkbaar snap je totaal niet wat er gezegd wordt? er wordt namelijk opgemerkt dat je website lekken bevat waardoor mensen zomaar je gehele website kunnen aanpassen zonder dat ze daarvoor de juiste rechten hebben. Juist doordat JIJ fouten maakt in je scripting.

Je moet er namelijk vanuit gaan dat bezoekers kwaad willen, aangezien jij je script hier aanbied om te gebruiken door andere gebruikers dien jij te zorgen dat je systeem geen liefst zo min mogelijk lekken bevat. Als men dus aangeeft dat je login totaal niet beveiligd is moet je dat aan het denken zetten en je CMS hierop aanpassen.

Dus eerstvolgende acties van jou kant zouden moeten zijn.
1 - Inputs controlleren
2 - Fouten oplossen.

een paar tips.

www.php.net/filter_var (controlleren van gegevens)
www.php.net/mysql_real_escape_string (voorkomen van SQL injecties)

Have fun en probeer een les te nemen uit de reacties ipv boos te worden!!!

<?=$nix;?>

-

Hier even een reactie op: Voor naam [ www.bruggema.nl ]

Zoals je misschien ziet is het systeem momenteel in de beta, zo kan het systeem fouten bevatten zoals SQL injecties. Ik ben bezig met een volgende versie waarbij ik dit ga oplossen, waarbij ik luister naar de tips die ik krijg van diverse zoals van jouw. Zo bedankt.

Maar ik bedoelde er mee te zeggen dat je alleen kan inloggen zonder wachtwoord etc. als je bevoegt bent om in de FTP te komen en PHP toe te voegen. Niet zomaar gewoon als bezoeker.

Vermeld hier alle zaken betreffende valkuilen, handige informatie, (installatie-)instructies e.d.

Volgens mij snap je het nog niet...

FTP staat hier los van...
Je script bevat "lekken"...


Mooie vergelijking:
Een computer kun je hacken doordat je pc niet veilig genoeg is... en dan hoef je er niet eens voor/achter te zitten... snap je het verschil.

Ik zou als ik jou was... nog maar is verdiepen in het taaltje... want ik vind persoonlijk dat je script ook er rommelig uit ziet...

Volgens mij bedoelt Marnix dat als je in het CMS wilt komen, je eerst de bestanden hiervoor upload, en als je klaar bent ze weer verwijderd van de server zodat niemand erin kan komen.

Maar toch heb ik ook liever dat je de fouten oplost, en gelukkig geef je al aan hiermee bezig te zijn ;)

.

-

Ja, zoiets. Momenteel ben ik nu (mogelijk) klaar met het beveiligen van het systeem. Nu nog een paar kleine bugs oplossen, een paar nieuwe pagina's en klaar. Dan release ik de 2de beta, en met de reacties daarop (indien er weer lekken gevonden worden, waarschijnlijk niet) ga ik weer verder met het systeem.

> Ja, zoiets. Momenteel ben ik nu (mogelijk) klaar met het beveiligen van het systeem.
> Nu nog een paar kleine bugs oplossen, een paar nieuwe pagina's en klaar. Dan release
> ik de 2de beta, en met de reacties daarop (indien er weer lekken gevonden worden,
> waarschijnlijk niet) ga ik weer verder met het systeem.

Ik durf gerust te zeggen van wel. Ook ik maak nog wel eens fouten in dat soort dingen, dat gebeurt. Door het script open source te zetten komen dat soort dingen aan het licht en kunnen mensen zoals ik er naar kijken om je foutjes te vinden. Je moet niet boos worden als iemand een foutje vind, het lijkt me een beter idee om gewoon je foutjes op te lossen ;)

Laat maar weten als de nieuwe versie online staat, dan ga ik wel kijken of ik lekken zie. En als je een online versie hebt, dan wil ik ook best bewijzen dat ik toegang tot het systeem kan krijgen zonder dat ik toegang tot je server heb.

Post hier de source-code van je script. Alle informatie tussen <? ... ?> en <?php ... ?> zal automatisch worden getoond in color-coding. 

Let op! Het is niet de bedoeling om hier een link naar je website te plaatsen. Post hier gewoon de code, veel simpeler, sneller en meer kans dat het blijft staan.

-

Hierbij wil ik vertellen dat ik zojuist het CMS geupdate heb op de volgende punten:

- Verschillende typefouten weggehaald
- Het systeem veiliger gemaakt tegen SQL injecties
- Indien geen nieuws berichten, een melding
- Reageren uit/aan per nieuws artikel
- Bij pagina bewerken, nu wel TinyMCE
- Changelog toegevoegd

Indien iemand weer een fout vind, zoals een injectie, gelieve het te vermelden.

Vermeld hier alle zaken betreffende valkuilen, handige informatie, (installatie-)instructies e.d.

Zomaar een tip:

gebruik voor datum geen varchar, maar datetime.
Handig voor statistieken..scheelt trouwens ook weer veel narigheid achteraf.

1. Waarom variabelen opslaan in een andere variabele? Het is onnodig, en kost extra geheugen om variabelen te kopieeren. Nergens voor nodig, je kan ook ook in je code direct $_POST['titel'] gebruiken. En het is nog overzichtelijker ook.

2. DATETIME is de beste manier om data's en tijden te gebruiken.

3. Waarom geen goede foutafhandeling gebruiken bij je query. De naamgeving is ook neit echt netjes, een $result voor de query lijkt me wat duidelijker en overzichtelijker, in je query stop je overigens een $sql.
Het gebruiken van die(), is gewoon kerkhoftactiek. Als jij iets fouts doet voor mij, dan zeg ik toch ook niet of je dood wilt vallen, maar zoek ik een goede oplossing (if-(elseif)-else)...
Bovendien stopt je script ook direct met uitvoeren met die(). WIL je dat? Dit kan zelf database(relatie)-corruptie veroorzaken.

4. Die isset met $_POST['bewerkres'], waarom controleren we gewoon niet op de nette manier met:
if($_SERVER['REQUEST_METHOD'] =="POST") ?
Veel netter, en logischer.

5. Staff kunnen elkaars rechten ontnemen. Dit lijkt me niet echt gewenst.

<?php
// (1) en (2) Onzinnig, en verkeerd gebruik van datum en tijd.
$titel = $_POST['titel']; 
$reageren = $_POST['reageren']; 
$bericht = $_POST['bericht']; 
$datum = date("d-m-Y H:i:s", time());
$naam = $_SESSION["naam"];

// (3) Foute foutafhandeling
$sqls = mysql_query($sql_query) or die(mysql_error()); 
echo 'Het nieuws bericht is met succes toegevoegt, ga <a href="nieuws.php">verder!</a>';

// (4) Verkeerde controle op verzenden (POST)
if(isset($_POST['bewerkres'])) {
}

?>

Waarom zit je te morrelen in php met datums?

Dat kan mysql zelf regelen ;)

Overal waar je een INSERT of UPDATE query doet zet je als waarde NOW() en je bent klaar.

Verder zou ik van reageren een tinyint maken met een default waarde.. Bijvoorbeeld 0 of 1.

if($row['reageren'] == 1)
{
//doe je ding
}

Heb alleen naar de install pagina gekeken.
Maar er zitten toch nog best veel verbeterpunten in.
En dan zou ik beginnen met de database.

Een wachtwoord codeer je ALTIJD .
Als je daarvoor md5() gebruikt, heb je een kolom VARCHAR nodig met 32 tekens ;)

Dus zou je het willen veranderen hier de nieuwe opzet van je database..

CREATE TABLE IF NOT EXISTS cat (
cat_id int(11) NOT NULL auto_increment,
cat_naam varchar(50) NOT NULL,
actief TINYINT NOT NULL default '1',
volg int(11) NOT NULL,
PRIMARY KEY (cat_id)
) ENGINE=MyISAM DEFAULT CHARSET=latin1 AUTO_INCREMENT=1 ;

CREATE TABLE IF NOT EXISTS nieuws (
nieuws_id int(11) NOT NULL AUTO_INCREMENT,
titel varchar(60) NOT NULL,
naam varchar(50) NOT NULL,
datum DATETIME,
bericht text NOT NULL,
reageren TINYINT DEFAULT '1',
PRIMARY KEY (nieuws_id)
) ENGINE=MyISAM DEFAULT CHARSET=latin1 AUTO_INCREMENT=1 ;

CREATE TABLE IF NOT EXISTS nieuws_reacties (
nieuwsre_id int(11) NOT NULL auto_increment,
nieuws_id int(11) NOT NULL,
naam varchar(50) NOT NULL,
reactie text NOT NULL,
datum DATETIME,
PRIMARY KEY (nieuwsre_id)
) ENGINE=MyISAM DEFAULT CHARSET=latin1 AUTO_INCREMENT=1 ;

CREATE TABLE IF NOT EXISTS pagina (
pagina_id int(11) NOT NULL auto_increment,
titel varchar(50) NOT NULL,
pagina text NOT NULL,
naam varchar(50) NOT NULL,
cat varchar(50) NOT NULL,
actief TINYINT NOT NULL default '1',
volg int(11) NOT NULL,
PRIMARY KEY (pagina_id)
) ENGINE=MyISAM DEFAULT CHARSET=latin1 AUTO_INCREMENT=1;

CREATE TABLE IF NOT EXISTS user (
id int(11) NOT NULL auto_increment,
naam varchar(50) NOT NULL,
email varchar(60) NOT NULL,
wachtwoord varchar(32) NOT NULL,
PRIMARY KEY (id)
) ENGINE=MyISAM DEFAULT CHARSET=latin1 AUTO_INCREMENT=1;

Denk dat je hiermee verder kunt gaan testen waar je wat verbetering uit kunt halen ;)

Succes met beta 3 ;)

<?php
// benoem de namen van het tabel, dat scheelt zoek werk ;)
$sql = "SELECT tabel,
               nogeentabele,
'              date_format(datum, '%d-%m-%Y %H:%i') AS datum_NL
               FROM
               blaat";

$result = mysql_query($sql);
$row = mysql_fetch_assoc($result);

echo $row['datum_NL'];
?>

<?php
$sql = "INSERT INTO user (wachtwoord
                         )
                         VALUES('".mysql_real_escape_string(md5($_POST['wachtwoord']))."')";
?>

-

En er is een nieuwe versie beschikbaar:

- Verschillende typefouten weggehaald
- Het systeem veiliger gemaakt tegen XXS injecties
- Doneren in adminstratie werkend
- Opties.PHP in admin/ werkt lokaal
- Categorieen van pagina's in admin paneel.
- Changelog pagina toegevoegd
- Mod-rewrite
- Kleine bug fixes

ik mis een paar punten wat ik belangrijk vindt in een cms
* thema`s (templates)
* modules/uitbreidingen
* meertalig