Link to home
Start Free TrialLog in
Avatar of Stacie
StacieFlag for United States of America

asked on

Trying to write a query that will prevent updating another record in C#VB

I have the following query and in the past another record was updated. I'm trying to find out what is the best way to prevent this. I feel this is not the best way to write an update statement.

The line in question that I have is    strSQL2="UPDATE LDA_CAN_ROLE_GAP set " & DBTABLE & "='" & NewText & "' where CR_ID='" & CRGID & "'"

	<tr><td> <FONT COLOR=RED><B>Edit Fields and hit "Save" to Update Candidates <%Response.write(Caption1)%> for this Role</B></FONT></td></tr>
<form method="POST" action="EditSkillGap.asp?UID=<%=UID%>&CID=<%=CID%>&CRGID=<%=CRGID%>&RID=<%=RID%>&PCID=<%=PCID%>&EID=<%=EID%>&DBTABLE=<%=DBTABLE%>">
<table  width=560 cellpadding=4 cellspacing=1   border=1  bordercolor="LightGrey" bordercolorlight="LightGrey" bordercolordark="White" align="left">
 <INPUT TYPE="hidden" NAME="UID" VALUE="<%=UID%>">
    <%sqlqt=Chr(39) %>
<%if Request.Form("UPD")="Y" then
    NewText=Request.Form("JDesc")
    NewText=Replace(NewText,sqlqt,sqlqt & sqlqt)
  [b]  strSQL2="UPDATE LDA_CAN_ROLE_GAP set " & DBTABLE & "='" & NewText & "' where CR_ID='" & CRGID & "'"[/b]
    adoRs1.Open strSQL2, strDSN
  
    UPDOK=1

    else
    end if %>
	

Open in new window

Avatar of zephyr_hex (Megan)
zephyr_hex (Megan)
Flag of United States of America image

Yikes.  This is prone to SQL Injection.
You should be using SqlCommand.ExecuteNonQuery with Parameters.

You should also consider separating your server side code from your HTML.  Separation of Concerns as a general coding practice.
I agree with Zephyr completely! And you are right, it is not the best way to write an update statement.

The first thing I would consider is Zephyr's advice. The way you have that setup, you can modify the UID value in the query string, paste it in the URL and possibly change the wrong UID.

LIke Zephyr stated, I would re-implement that page and move the code to code-behind instead of putting it inline with the markup. Then, if you have access to modify the database, turn that Update statement into a parameterized stored procedure. That practically eliminates the SQL Injection vulnerability in the page, and it guarantees that the only record updated is the one you specify.

Another reason you may be updating more than one record is if there is an UPDATE trigger on that table. If so, it could be the table's trigger is being fired and is updating another record.
This question needs an answer!
Become an EE member today
7 DAY FREE TRIAL
Members can start a 7-Day Free trial then enjoy unlimited access to the platform.
View membership options
or
Learn why we charge membership fees
We get it - no one likes a content blocker. Take one extra minute and find out why we block content.