1

I have the table and fields created on the SQL end. The debugger points to the cnn.Execute uSQL being the issue. I am trying to write the user and computer name of who is accessing the sheet to SQL.

Sub UpdateTable()

Dim cnn As ADODB.Connection
Dim uSQL As String
Dim strText As String
Dim strDate As Date
Dim strUsername As String
Dim strComputerName As String

strUsername = Environ("username")
strComputerName = Environ("Computername")


Set cnn = New Connection
cnnstr = "Provider=SQLOLEDB; " & _
        "Data Source=icl-analive; " & _
        "Initial Catalog=DW_ALL;" & _
        "User ID=dw_all_readonlyuser;" & _
        "Trusted_Connection=Yes;"

cnn.Open cnnstr

uSQL = "INSERT INTO Audit (UN,CN) VALUES StrUsername , strComputer"

Debug.Print uSQL

cnn.Execute uSQL
cnn.Close
Set cnn = Nothing
Exit Sub
End Sub
Chris
  • 323
  • 5
  • 21
  • The SQL user name for logging in to your SQL sever is "dw_all_readonlyuser", so, could it be that under this user you do not have `insert` privileges in the database? – Carsten Massmann Nov 15 '16 at 19:28
  • 1
    FWIW I find it rather jarring that you name `UpdateTable` a procedure that performs an `INSERT`, not an `UPDATE` as one would expect by reading a method call. – Mathieu Guindon Nov 15 '16 at 19:45
  • 1
    You should be using parameterized queries for this sort of thing before bobby tables comes to visit. http://bobby-tables.com Here is a great example. http://stackoverflow.com/questions/10352211/vba-ado-connection-and-query-parameters – Sean Lange Nov 15 '16 at 19:46
  • 1
    `uSQL = "INSERT INTO Audit (UN, CN) VALUES (?, ?)"` (notice the parens) - and then use an `ADODB.Command` and parameterize that baby. – Mathieu Guindon Nov 15 '16 at 19:56

1 Answers1

-1

Your uSQL should be like this:

uSQL = "INSERT INTO Audit (UN,CN) VALUES " & StrUsername & "," & strComputer

Edit: Actually like this:

INSERT INTO Audit (UN,CN) VALUES '" & StrUsername & "','" & strComputer & "'"

The difference is that we should set the strings in inverted commas for the SQL. Thanks @Tony Dong in the comments.


This answer is a classic example of a vulnerable code for SQL injection.In order to prevent it, consider using parameterized queries as in this example:

VBA, ADO.Connection and query parameters

Vityata
  • 42,633
  • 8
  • 55
  • 100
  • 1
    You have to use single quote uSQL = "INSERT INTO Audit (UN,CN) VALUES '" & StrUsername & "','" & strComputer & "'" – Tony Dong Nov 15 '16 at 19:27
  • TonyDong, you are right. @Chris - Tony's solution should work. – Vityata Nov 15 '16 at 19:30
  • Please do NOT use like this. This needs to be parameterized. When you build up a string like this it is wide open to sql injection. Not mention that if the username or computer has a single quote in it this will fail miserably. – Sean Lange Nov 15 '16 at 19:41
  • We are speaking about VBA here. Unfortunately, this is standard in the language. It is opened for SQL injection, yes. But if you want to make SQL injection in VBA, most probably you can simply take the username and the password and inject anything you want in the DB. – Vityata Nov 15 '16 at 19:45
  • @SeanLange and concerning the apostrophe - I suppose that you have never coded in VBA. Thus, you should have known what you are speaking about... – Vityata Nov 15 '16 at 19:47
  • I understand it is VBA and not using parameterized queries is just awful. And NO, VBA is no more vulnerable to sql injection than any other language. They are all vulnerable when you don't code them correctly. – Sean Lange Nov 15 '16 at 19:47
  • Actually I have coded quite a bit of VBA. And parameterizing queries is pretty simple to do. – Sean Lange Nov 15 '16 at 19:49
  • Ok, so how should it be? Tell me, I want to learn something new as well :) – Vityata Nov 15 '16 at 19:49
  • 2
    See my comment with a great example of how to parameterize queries in VBA. It works much like most other languages. – Sean Lange Nov 15 '16 at 19:50
  • I learnt something. I am using similar things in Python, in VBA I did not know that they exist. – Vityata Nov 15 '16 at 19:54
  • Awesome. Update your answer and my negative will happily become a positive. :D – Sean Lange Nov 15 '16 at 19:57