hvordan udgår jeg sql injections på denne kode

strSQL = "Select pris From pris Where pris = '" & replace(Request.Form("pris"),"'","''") & "';"

set rst = DataConn.Execute (strSQL)

if rst.bof and rst.eof then
  strSQL = "Insert into pris (pris) values('" & replace(Request.Form("pris"),"'","''") & "')"
  DataConn.Execute(strSQL)
  Alert = "Prisen er nu oprettet "
else
  Alert = "Du har allerede oprettet den pris, prøv igen"
end if
end if
%>

smukt..

kan du lave den første i denne lange her så kan jeg selv resten :-)

strSQL = "Insert into tilmelding (efternavn, forfos1, for2, forfos2, adresse, postnr, fby, telefon, email, fornavn, dato, holdnr, pris) values('" & Request.Form("efternavn") & "','" & Request.Form("forfos1") & "','" & Request.Form("for2") & "','" & Request.Form("forfos2") & "','" & Request.Form("adresse") & "','" & Request.Form("postnr") & "','" & Request.Form("fby") & "','" & Request.Form("telefon") & "','" & Request.Form("email") & "','" & Request.Form("fornavn") & "','" & Request.Form("dato") & "','" & holdnummer & "','" & pris & "'  )"
DataConn.Execute(strSQL)

og et lille svar :-) genialt tak

...men er pris ikke numerisk i databasen? Hvis den er, så bør du checke for dette inden du benytter værdien i din SQL-sætning.

if isnumeric(Request.Form("pris")) then

  strSQL = "Select pris From pris Where pris = " & Request.Form("pris") & ";"

  set rst = DataConn.Execute (strSQL)

  if rst.bof and rst.eof then
    strSQL = "Insert into pris (pris) values(" & Request.Form("pris") & ")"
    DataConn.Execute(strSQL)
    Alert = "Prisen er nu oprettet "
  else
    Alert = "Du har allerede oprettet den pris, prøv igen"
  end if
end if
end if
%>

Generelt set er det en bedre idé at benytte command-objektet og parametre i stedet for denne type SQL-strenge, men det er en lidt anden historie...

ok det kigger jeg lige på .-)

strSQL = "Insert into tilmelding " & _
    "(efternavn, forfos1, for2, forfos2, adresse, " & _
    "postnr, fby, telefon, email, fornavn, dato, holdnr, pris) " & _
    "values('" & replace(Request.Form("efternavn"),"'","''") & "'," & _
    "'" & replace(Request.Form("forfos1"),"'","''") & "'," & _
    "'" & replace(Request.Form("for2"),"'","''") & "'," & _
    "'" & replace(Request.Form("forfos2"),"'","''") & "'," & _
    "'" & replaec(Request.Form("adresse"),"'","''") & "'," & _
    "'" & replace(Request.Form("postnr"),"'","''") & "'," & _
    "'" & replace(Request.Form("fby"),"'","''") & "'," & _
    "'" & replace(Request.Form("telefon"),"'","''") & "'," & _
    "'" & replace(Request.Form("email"),"'","''") & "'," & _
    "'" & replace(Request.Form("fornavn"),"'","''") & "'," & _
    "'" & replace(Request.Form("dato"),"'","''") & "'," & _
    "'" & replace(holdnummer,"'","''") & "'," & _
    "'" & replace(pris,"'","''") & "')"
DataConn.Execute(strSQL)

den siger :
Error Type:
Sun ONE ASP VBScript runtime (0x800A000D)
Type mismatch


i den kode du lige har givet mig .-)

Med et commandobjekt kan du gøre således:

strSQL = "Insert into tilmelding " & _
    "(efternavn, forfos1, for2, forfos2, adresse, " & _
    "postnr, fby, telefon, email, fornavn, dato, holdnr, pris) " & _
    "values(?,?,?,?,?,?,?,?,?,?,?,?,?)"

set cmd = Server.CreateObject("ADODB.Command")
set cmd.ActiveConnection = DataConn
cmd.CommandType = adCmdText
cmd.CommandText = strSQL
cmd(0) = Request.Form("efternavn")
cmd(1) = Request.Form("forfos1")
cmd(2) = Request.Form("for2")
cmd(3) = Request.Form("forfos2")
cmd(4) = Request.Form("adresse")
cmd(5) = Request.Form("postnr")
cmd(6) = Request.Form("fby")
cmd(7) = Request.Form("telefon")
cmd(8) = Request.Form("email")
cmd(9) = Request.Form("fornavn")
cmd(10) = Request.Form("dato")
cmd(11) = holdnummer
cmd(12) = pris
cmd.Execute

og så skal du lige lægge denne metadata-linie ind i toppen af din asp-side (eller endnu bedre, i global.asa, da du så slipper for at tænke på den på resten af sitet).

<!-- METADATA TYPE="typelib" uuid="00000205-0000-0010-8000-00AA006D2EA4"  -->

Det er en reference til ADO Type Library, som giver dig adgang til de tekstuelle version af enumerationer (f.eks. adCmdText)

Virkede den før tilføjelsen af replace...? ...og hvilken database er der tale om?

ja den virkede før...-)

nu bliver den sur på denne linie : cmd.CommandText = strSQL

Error Type:
Sun ONE ASP VBScript runtime (0x80070057)
/klubadmin/hjemmeside/tilmelding.asp, line 66


Browser Type:
Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; SV1)

Page:
POST 131 bytes to /klubadmin/hjemmeside/tilmelding.asp

POST Data:
id=14&efternavn=asdfasdf&forfos1=asdaskl&fornavn=l&dato=l&adresse=l&postnr=l&fby=l&telefon=l&email=perree@kriegbaum.dk&send=Tilmeld

Time:
Tuesday, June 06, 2006, 9:57:58 AM


More information:
Sun ONE Active Server Pages Support

Hmm... noget kunne tyde på at Sun ONE ASP ikke understøtter CommandText for command-objektet... så er spørgsmålet om man kan finde en dokumentation for Sun ONE ASP, så man kan finde ud af hvad den rent faktisk understøtter. Det jeg har vist fungerer i det mindste på en IIS med ASP, men det hjælper jo ikke dig videre...

Mht. fejlen i den tekstuelle version af SQL-sætningen, så kan du evt. undersøge om der skulle være nogle af værdierne som ikke er numeriske i dit input (f.eks. postnr og pris) og hvor databasen forventer at de er numeriske. Det samme gælder for dato... I det hele taget bør du lave en validering af indholdet i formularen inden du forsøger at indsætte data i databasen, for at fange evt. fejl så tidligt som muligt (det sparer tid og resurser).

ok tak, men kan jeg ikke sådan lige nu lave denne om så den virker:

'" & Request.Form("forfos1") & "'

Jo, forudsat at det i databasen er en tekst, så burde dette fungere:

'" & replace(Request.Form("forfos1"),"'","''") & "'

men igen kan det være at databasen (hvad end det så er for en) forventer en anden form for strengafgrænser (f.eks. ´ i stedet for ''), så det kan være denne fungere i stedet:

'" & replace(Request.Form("forfos1"),"'","´") & "'

Dog synes jeg den første burde være OK, da den jo fungerer uden replace-faciliteten...

jeg prøver lige det er en mysql hos b-one.dk

Tak for point :)

tak for god kode :-)

jeg har lidt problemmer jeg kan ikke få denne linie til at virke

strSQL = "select count(*) as antalTilmeldte from tilmelding where holdnr = '" & rs("holdnr") & "'"

Hvad er det der ikke fungerer? Hvilken fejl får du (hvis du får en)?

Kan det evt. være at holdnr er af typen tal i databasen og at du derfor skal bruge denne kode i stedet for:

strSQL = "select count(*) as antalTilmeldte from tilmelding where holdnr = " & rs("holdnr")

det er der problemmet er når jeg opretter et hodl med holdnr = '  så laver den Error Type:
ADODB.Recordset.1 (0x80004005)
SQLState: 4 Native Error Code: 1064 [TCX][MyODBC]You have an error in your SQL syntax. Check the manual that corresponds to your MySQL server version for the right syntax to use near ''''' at line 1
denne fejl

jeg leder efter et godt login system hvor man også kan oprettet brugere osv..

og imun overfor sql injections osv

bare det bedste.. det må gerne koste lidt:-) kender du et til mysql og asp

når hvad siger du til det :-)

Jeg kender ikke lige noget login system (hvad end det så er :))

Hvad angår den fejl du skriver, hvad er det så rent faktisk du skriver, for den meddelelse lyder da noget spøjs, hvis der end ikke er nogen apostrof i SQL-sætningen...

Prøv evt. at skrive SQL-sætningen ud inden du udfører den (evt. efterfulgt af en Response.Flush, så den når at blive udskrevet inden siden fejler).

login system he he altså opret en bruger der har adgang til et system.. ligesom her på eksperten...

Jamen jeg har rettet al min kode til så jeg kan taste et  '  ind i alle felter uden databasen går ned.. og det er jo din gode kode jeg bruger der..

men når jeg opretter et hold hvor jeg taster ' ind i holdnr feltet laver siden fejlen som jeg viser ovenover...


ADODB.Recordset.1 (0x80004005)
SQLState: 4 Native Error Code: 1064 [TCX][MyODBC]You have an error in your SQL syntax. Check the manual that corresponds to your MySQL server version for the right syntax to use near ''''' at line 1
denne fejl

OK, det lyder som en god test... med mindre du forsøger at gemme en apostrof i et talfelt for det skal du validere for inden du indsætter data i din database :) - talfelter skal være numeriske (eller NULL hvis dette tillades for det enkelte felt) eller vil databasen brokke sig, så det skal du lige be- eller afkræfte først.

Hvis holdnr er et tekstfelt så skal du blot bruge samme fremgangsmåde som vist tidligere, nemlig

strSQL = "select count(*) as antalTilmeldte from tilmelding where holdnr = '" & replace(rs("holdnr")&"", "'","´") & "'"

ja det er sgu da rigtigt.. hvordan sætter jeg en validering ind om at holdnr skal være et tal ?

strSQL = "Select holdnr From hold Where holdnr =  '" & replace(Request.Form("holdnr"),"'","''") & "';"

set rst = DataConn.Execute (strSQL)

if rst.bof and rst.eof then
  strSQL = "Insert into hold (holdnavn, instruktør, holdnr, holdantal, tider, sluttid, pris, dag) values('" & replace(Request.Form("holdnavn"),"'","''") & "','" & replace(Request.Form("instruktør"),"'","''") & "','" & replace(Request.Form("holdnr"),"'","''") & "','" & replace(Request.Form("holdantal"),"'","''") & "','" & replace(Request.Form("tider"),"'","''") & "','" & replace(Request.Form("sluttid"),"'","''") & "','" & replace(Request.Form("pris"),"'","''") & "','" & replace(Request.Form("dag"),"'","''") & "')"


  DataConn.Execute(strSQL)
  Alert = "Holdnummer er nu oprettet "
else
  Alert = "Du har allerede oprettet det holdnummer, prøv igen"
end if
end if

Umiddelbart kan du benytte funktionen isNumeric til at checke om en værdi er numerisk. Det skal du så gøre inden der gemmes noget som helst i databasen. F.eks.:

if isNumeric(Request.Form("holdnr")&"") then
  strSQL = "Select holdnr From hold Where holdnr = " & Request.Form("holdnr") & ";"

  set rst = DataConn.Execute(strSQL)

  if rst.eof then
    strSQL = "Insert into hold (holdnavn, instruktør, holdnr, holdantal, tider, sluttid, pris, dag) values('" & replace(Request.Form("holdnavn"),"'","''") & "','" & replace(Request.Form("instruktør"),"'","''") & "'," & Request.Form("holdnr") & ",'" & replace(Request.Form("holdantal"),"'","''") & "','" & replace(Request.Form("tider"),"'","''") & "','" & replace(Request.Form("sluttid"),"'","''") & "','" & replace(Request.Form("pris"),"'","''") & "','" & replace(Request.Form("dag"),"'","''") & "')"

    DataConn.Execute(strSQL)
    Alert = "Holdnummer er nu oprettet "
  else
    Alert = "Du har allerede oprettet det holdnummer, prøv igen"
  end if
end if

Når du nu er igang med at finde numeriske felter, mon så ikke også holdantal er et tal...? Hvis jo, så kan du jo lige øve dig på det felt (se hvad jeg har gjort ifht. feltet holdnr - det samme skal i så fald gøres ved holdantal)...

ok men skal den ikke have er respones = holdnummer skal være et tal..

holdantal tager den fra en dropdown jeg har lavet kun med tal..

Det er ville være en god idé (regnede dog med at du selv lige kunne smide relevante fejlbeskeder på). Det er altid en god idé at validere input, selvom du har afgrænset brugerens mulighed til at vælge fra en drop down - det kunne jo være der var en som fik den snedige idé at poste deres egen version af din form til serveren og så er du jo ikke sikker på at værdien er numerisk længere (trust nobody! :))...

ok ja du har ret.. men jeg er ikke så skrap til det med response så hvis du lige har to sek må du meget gerne sætte det ind tak :-)

Jeg antager at du har et system kørende til udskrivning af dine Alert-variabler, så jeg benytter bare samme fremgangsmåde:

if isNumeric(Request.Form("holdnr")&"") then
  strSQL = "Select holdnr From hold Where holdnr = " & Request.Form("holdnr") & ";"

  set rst = DataConn.Execute(strSQL)

  if rst.eof then
    strSQL = "Insert into hold (holdnavn, instruktør, holdnr, holdantal, tider, sluttid, pris, dag) values('" & replace(Request.Form("holdnavn"),"'","''") & "','" & replace(Request.Form("instruktør"),"'","''") & "'," & Request.Form("holdnr") & ",'" & replace(Request.Form("holdantal"),"'","''") & "','" & replace(Request.Form("tider"),"'","''") & "','" & replace(Request.Form("sluttid"),"'","''") & "','" & replace(Request.Form("pris"),"'","''") & "','" & replace(Request.Form("dag"),"'","''") & "')"

    DataConn.Execute(strSQL)
    Alert = "Holdnummer er nu oprettet "
  else
    Alert = "Du har allerede oprettet det holdnummer, prøv igen"
  end if
else
  Alert = "Du skal angive holdnr som et tal"
end if

NEMLIG..

Error Type:
Sun ONE ASP VBScript compilation (0x800A03F6)
Expected 'End'
/klubadmin/oprethold.asp, line 361, column 25




linie 361 Loop
end if
Response.Write "</table>"

prøv at smide en ekstra end if ind i bunden af den kodeblok jeg viste (den forsvandt jo fra den kode jeg tog udgangspunkt i).

if isNumeric(Request.Form("holdnr")&"") then
  strSQL = "Select holdnr From hold Where holdnr = " & Request.Form("holdnr") & ";"

  set rst = DataConn.Execute(strSQL)

  if rst.eof then
    strSQL = "Insert into hold (holdnavn, instruktør, holdnr, holdantal, tider, sluttid, pris, dag) values('" & replace(Request.Form("holdnavn"),"'","''") & "','" & replace(Request.Form("instruktør"),"'","''") & "'," & Request.Form("holdnr") & ",'" & replace(Request.Form("holdantal"),"'","''") & "','" & replace(Request.Form("tider"),"'","''") & "','" & replace(Request.Form("sluttid"),"'","''") & "','" & replace(Request.Form("pris"),"'","''") & "','" & replace(Request.Form("dag"),"'","''") & "')"

    DataConn.Execute(strSQL)
    Alert = "Holdnummer er nu oprettet "
  else
    Alert = "Du har allerede oprettet det holdnummer, prøv igen"
  end if
else
  Alert = "Du skal angive holdnr som et tal"
end if
end if

ja den rettede selv det virker jubii :-)

Goodie! :)

hvad bruger jeg hvis if isNumeric(Request.Form("holdnr")&"") then
ikke er et tal men bare et text felt.. alstå de skal skrive noget i feltet.... det må ikke være blank?

if len(Request.Form("feltnavn")) > 0 then
  ' fortsæt validering og gemme i databasen
else
  Alert = "Feltet 'feltnavn' må ikke være tomt"
end if

feltnavn er naturligvis navnet på det formularfelt du vil validere...

forfanden jeg bliver bare klog her.. du er super skal jeg ikke lige smide lidt point til dig ?

Nej, det skal du ikke tænke på - gem dem til en anden gang :)

ok men jeg skylder :-) og jeg er sikkert snart tilbage igen.. kender du slet ikke et god system jeg kan bruge til brugeradmin.. ?

nu er den ikke god.. her skal jeg rette men den skriver alerten med det samme ?

if isNumeric(Request.Form("tider")&"") then
if request.form("update") <> "" then

strSQL = "update starttid set tider='" & replace(Request.Form("tider"),"'","''") & "' where id = " & request.querystring("id")
Set rs = DataConn.Execute(strSQL)

Alert = "Starttiden er nu opdateret "

end if
else
  Alert = "Du skal angive starttid som et tal"

end if
strSQL = "Select * From starttid Where Id = " & Request.Querystring("id")
Set rs = DataConn.Execute(strSQL)

Ikke som sådan. Jeg plejer at håndkode det jeg skal bruge. Det nærmeste jeg kan komme sådan noget er et CMS-system, men der har jeg ikke erfaring med noget der kører på ASP og MySQL - så der kan jeg desværre ikke hjælpe...

07/06-2006 23:24:55 >> Jeg er ikke helt sikker på at jeg forstår hvad du mener. Hvis du vil have udskrevet beskeden i stedet for at tildele den variablen Alert, kan du bare udskifte teksten "Alert = " med "Response.Write ", altså:

if isNumeric(Request.Form("tider")&"") then
  if request.form("update") <> "" then
    strSQL = "update starttid set tider='" & replace(Request.Form("tider"),"'","''") & "' where id = " & request.querystring("id")
    Set rs = DataConn.Execute(strSQL)

    Response.Write "Starttiden er nu opdateret "
  end if
else
  Response.Write "Du skal angive starttid som et tal"
end if

hmm den bliver ved med at skrive "Du skal angive starttid som et tal" når man skal rette.. den skal først skrive det når man har rettet tallet

er tider ikke en dato?

nej det er kl. 12.00

OK, det kan vist også kategoriseres som en dato :)

så kan du prøve at checke med isDate i stedet for isNumeric.

men det virker jo når man skal oprette et starttid.. den kan jo validerede det men den viser alert teksten med det samme?

så må det være fordi koden der udskriver linien er placeret forkert i din if-struktur. Prøv lige at checke om det kan være årsagen...

ja du har ret men jeg kan ikke lige se om den står forkert ..

if isNumeric(Request.Form("pris")&"") then

if request.form("update") <> "" then
strSQL = "update pris set pris='" & replace(Request.Form("pris"),"'","''") & "' where id = " & request.querystring("id")
Set rs = DataConn.Execute(strSQL)

Alert = "Prisen er nu opdateret "
end if
else
  Alert = "Du skal angive pris som et tal"

end if
strSQL = "Select * From pris Where Id = " & Request.Querystring("id")
Set rs = DataConn.Execute(strSQL)

I den der stump kode ser der ikke ud til at være nogen problemer med stukturen, så på det grundlag kan jeg ikke lige se hvad der skulle være galt...