April 13, 2015 at 3:15 am
Hello, i need help about my APS page and SQL.
I have a ASP page with forms. I need to save value forms in a table. ok, everything is ok.
so, my problem is:
- I need to ensure that the writing on the table is unique. No one can write anything while another transaction is complete.
- first, i need to check if "conta" (account number) is valid
- then, i need to check if i can save the value, max value to save is 25 000 000 € so i need to see request.form("montante")...
- id everything is ok, i insert the values in table
Initially , I wrote the following code :
.......if request.form("cmdSubmit")="Save" then
SQLStmtConta = "SELECT conta,nome,bg FROM sig.dbo.TblClientes where conta = '" &request.form("conta")& "'"
set rsconta = Conn.execute(SQLStmtConta)
if not rsconta.eof then
nomecliente= rsconta("nome")
bg= rsconta("bg")
SQLStmtCheck = "select isnull(sum(montante),0) as montante_subscrito, 25000000-isnull(sum(montante),0) as valorpossivel from worktransation.dbo.DP_Estruturado_032015"
set rscheck = Conn.execute( SQLStmtCheck )
if cdbl(request.form("montante")) > rscheck("valorpossivel") then
response.write "not possible to save this value. Max value is over..."
else
Conn.execute("INSERT INTO worktransation.dbo.DP_Estruturado_032015 (bg, conta, nome, montante, datareg, bnfreg, nomereg) " &_
" VALUES ('" & bg & "' ," &request.form("conta")& ", '" & nomecliente & "' "&_
" ,"& request.form("montante") &", getdate(), '"&session("numeroempregado")&"', '"&session("nome")&"') ")
end if
set rscheck = Conn.execute( SQLStmtCheck )
else
%>
<script>alert("Conta inválida")</script>
<%
end if
........
but, then I changed to this:
.......if request.form("cmdSubmit")="Save" then
SQLStmtConta = "SELECT conta,nome,bg FROM sig.dbo.TblClientes where conta = '" &request.form("conta")& "'"
set rsconta = Conn.execute(SQLStmtConta)
if not rsconta.eof then
nomecliente= rsconta("nome")
bg= rsconta("bg")
SQLStmtCheck = "SET TRANSACTION ISOLATION LEVEL SERIALIZABLE; " &_
"SET NOCOUNT ON; "&_
"begin tran t1; declare @id integer; " &_
"set @id = (select case when "& cdbl(request.form("montante")) &" > 25000000-isnull(sum(montante),0) then 0 else 1 end " &_
"from worktransation.dbo.DP_Estruturado_032015); " &_
"select @id as id; " &_
"if (select @id) = 1 " &_
"begin " &_
"INSERT INTO worktransation.dbo.DP_Estruturado_032015 (bg, conta, nome, montante, datareg, bnfreg, nomereg) " &_
"VALUES ('" & bg & "' ," &request.form("conta")& ", '" & nomecliente & "' "&_
" ,"& request.form("montante") &", getdate(), '"&session("numeroempregado")&"', '"&session("nome")&"'); " &_
"end " &_
"commit tran t1;"
set rscheck = Conn.execute( SQLStmtCheck )
id = rscheck("id")
if id=0 then
%>
<script>alert("Not possible to insert value, montante is bigger than 25000000 €")</script>
<%end if
if id=1 then
%>
<script>alert("inserted")</script>
<%end if
else
%>
<script>alert("Conta inválida")</script>
<%
end if
........
i think this second choice is good because put all the things in transation isolation....
but, will be right ?
thank you!
April 13, 2015 at 3:21 am
The very first thing you need to do is read up on SQL Injection, because your code is riddled with security vulnerabilities (unless you want your company's data exposed the way Sony's was).
I would strongly suggest you move the code to do the data work into a stored proc and call the stored proc from the ASP page. Once you've done that, you need to wrap it in a transaction and add the UPDLOCK and HOLDLOCK hints to the select, as currently there's nothing preventing multiple users from selecting the same row and then all doing the same insert. Also drop the SERIALISABLE isolation level, as it won't prevent multiple people reading the same row and it's likely to cause deadlocks
Please, fix the SQL Injection vulnerabilities first. Currently a malicious user can read the entire client's table (all rows) from this form, probably even change the data any way they want.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
April 13, 2015 at 4:47 am
Hello,
"
The very first thing you need to do is read up on SQL Injection, because your code is riddled with security vulnerabilities (unless you want your company's data exposed the way Sony's was).
"
My app works in close environment (intranet), but how can i be more secure? Can you give one example?
it is on "select * from tbl where c=request.form() " ???
i must not use request.form?
i must learn SQL injection...
"
I would strongly suggest you move the code to do the data work into a stored proc and call the stored proc from the ASP page. "
Can i call stored procedure with ASP variables? I need to check "request.form("montante")", "session()"...
"Once you've done that, you need to wrap it in a transaction and add the UPDLOCK and HOLDLOCK hints to the select, as currently there's nothing preventing multiple users from selecting the same row and then all doing the same insert. Also drop the SERIALISABLE isolation level, as it won't prevent multiple people reading the same row and it's likely to cause deadlocks"
so, my transation is not isolated?!?! grrrrrrrr....
i must read more about this, but i understand UPDLOCK...
Thank you!
April 13, 2015 at 4:59 am
Please go and do a lot of reading on the SQL injection vulnerability. And the fact that it's intranet changes nothing, it's vulnerable, you need to fix it.
There's lots of detailed resources on SQL injection, do some reading.
Your transaction is isolated, changes one person makes can't affect any other, however that's not enough in this case as you don't want two people to read the same row at the start, and by default SQL allows any number of people to read the same row. Isolation is about preventing people reading from seeing other changes, not preventing two people from reading the same data
Yes, you can call a procedure and pass parameters to it. As part of the reading on SQL Injection you should find plenty of good examples.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
April 13, 2015 at 5:31 am
"Your transaction is isolated, changes one person makes can't affect any other, however that's not enough in this case as you don't want two people to read the same row at the start, and by default SQL allows any number of people to read the same row. Isolation is about preventing people reading from seeing other changes, not preventing two people from reading the same data"
So...the second or other people can read bad data...hum...ok...
tanhk you
April 13, 2015 at 5:35 am
GilaMonster (4/13/2015)
Please go and do a lot of reading on the SQL injection vulnerability. And the fact that it's intranet changes nothing, it's vulnerable, you need to fix it.
+1000
April 13, 2015 at 5:38 am
jorge_gomes98 (4/13/2015)
So...the second or other people can read bad data...hum...ok...
No, that's not what I said.
Transactions and isolation levels prevent the second, third, etc people from reading data that the first person has changed until the transaction is complete.
What they do NOT prevent is multiple people reading a row and then each of them trying to change it. They'll all succeed in reading the same row, only one at time can change it, depending on the isolation level they may succeed or they may get a deadlock. Neither is a desirable outcome.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
April 13, 2015 at 5:57 am
i am sorry, i understand you, but my english is too bad!!!!
April 13, 2015 at 6:06 am
Viewing 9 posts - 1 through 8 (of 8 total)
You must be logged in to reply to this topic. Login to reply