iusql fails if the database password contains a semicolon

[hughmcmaster]

A Debian user reported a bug against iusql in the unixodbc package. It seems iusql is not escaping the semicolon character when used in connection strings.

The semicolon is usually a delimeter in connection strings.

I expect other special characters will trigger similar behaviour as noted in the bug report (copied below).

I was trying to connect to an MSSQL database using unixodbc/iusql with
the FreeTDS driver. The password for the login was randomly generated
and contained a semicolon ";" in it. This worked fine when using the
FreeTDS tools tsql and fisql. However, the iusql tool from unixodbc
failed as follows:

$ iusql myDSN myLogin 'Y&MI%4VtL?C@OuUwmWkksL;+!#V$JSo6' -v
[FreeTDS][SQL Server]Unable to connect to data source
[FreeTDS][SQL Server]Login failed for user 'myLogin'.
[ISQL]ERROR: Could not SQLDriverConnect

Changing the password in the database worked around the issue, but I
guess iusql needs to do better escaping of special characters in the
password.

[lurcher]

On 14/07/2022 02:54, Hugh McMaster wrote: A Debian user reported <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1014603> a bug against |iusql| in the |unixodbc| package. It seems |iusql| is not escaping the semicolon character when used in connection strings. The semicolon is usually a delimeter in connection strings. I expect other special characters will trigger similar behaviour as noted in the bug report (copied below). I was trying to connect to an MSSQL database using unixodbc/iusql with the FreeTDS driver. The password for the login was randomly generated and contained a semicolon ";" in it. This worked fine when using the FreeTDS tools tsql and fisql. However, the iusql tool from unixodbc failed as follows: $ iusql myDSN myLogin ***@***.***;+!#V$JSo6' -v [FreeTDS][SQL Server]Unable to connect to data source [FreeTDS][SQL Server]Login failed for user 'myLogin'. [ISQL]ERROR: Could not SQLDriverConnect Changing the password in the database worked around the issue, but I guess iusql needs to do better escaping of special characters in the password. — Reply to this email directly, view it on GitHub <#113>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABYK62K4G4JLBX2G5AVJCRTVT5XPDANCNFSM53QTU35A>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

Hmm, This is probably happening because iusql uses the dsn user and password and creates a connection string: "DSN=supplieddsn;UID=supplieduser;PWD=suppliedpassword" Which is then passed to SQLDriverConnect. So its clear that the semicolon will cause a problem. Off the top of my head I am not sure if there is a escape mechanism that should be used here to allow this to work. isql uses SQLConnect so won't have the same problem.

[TallTed]

@lurcher — I researched this a while ago for a similar issue, and can't quickly locate the answer I wrote about it at the time, but this StackOverflow answer on the same topic may provide you enough info to quickly patch iusql and any other samples that use SQLDriverConnect() with a simply constructed connection string. TL;DR: Reserved characters in DSN/UID/PWD values in connection string are escaped by wrapping each value (usually only PWD includes such reserved characters, but values without reserved characters may also be safely wrapped, so there's almost no reason to test before wrapping) in braces and doubling any internal close-brace (the one reason you might test before wrapping, but easier to always double this char within the wrapper than to individually address all the special cases).

[lurcher]

On 12/10/2022 15:18, Ted Thibodeau Jr wrote: @lurcher <https://github.com/lurcher> — I researched this a while ago for a similar issue, and can't quickly locate the answer I wrote about it at the time, but this StackOverflow answer <https://stackoverflow.com/a/42282362/241164> on the same topic may provide you enough info to quickly patch |iusql| and any other samples that use |SQLDriverConnect()| with a simply constructed connection string. TL;DR: Reserved characters in DSN/UID/PWD values in connection string are escaped by wrapping each value (usually only PWD includes such reserved characters, but values without reserved characters may also be safely wrapped, so there's almost no reason to test before wrapping) in braces /and/ doubling any internal close-brace (the one reason you might test before wrapping, but easier to always double this char within the wrapper than to individually address all the special cases). — Reply to this email directly, view it on GitHub <#113 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABYK62OOS7HMU3LQSAX4TZTWC3CDXANCNFSM53QTU35A>. You are receiving this because you were mentioned.Message ID: ***@***.***>

I am guessing that you suggest that in iusql I should just add {} around the password. That would make some sense if I knew that the "This appears to be a totally undocumented, despite several MSDN sources outlining the enclosing" behavior was known about by all driver writers. If not I will be breaking it for using iusql with those drivers. It may just be simpler if you enclose the password on the command line in {}.

[TallTed]

@lurcher — I found the results of my prior research, which I think makes plain the fully documented (though possibly hard to understand) solution and links to the relevant Microsoft ODBC docs.

[lurcher]

On 15/10/2022 21:45, Ted Thibodeau Jr wrote: @lurcher <https://github.com/lurcher> — I found the results of my prior research <https://stackoverflow.com/a/55151212/241164>, which I think makes plain the /fully documented/ (though possibly hard to understand) solution and links to the relevant Microsoft ODBC docs. — Reply to this email directly, view it on GitHub <#113 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABYK62PIL7OG2OIYZOFGSI3WDMJWBANCNFSM53QTU35A>. You are receiving this because you were mentioned.Message ID: ***@***.***>

That looks conclusive, It will also need checking in SQLDriverConnect(W) to make sure the extraction of the driver follows the rules.

[v-chojas]

It is probably better to add the quoting only if there are special characters that need it.

[lurcher]

On 17/10/2022 16:17, v-chojas wrote: It is probably better to add the quoting only if there are special characters that need it. — Reply to this email directly, view it on GitHub <#113 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABYK62MVOO5AZBPDJM6GZJDWDVU2DANCNFSM53QTU35A>. You are receiving this because you were mentioned.Message ID: ***@***.***>

That was my thought. I don't want to create problems in drivers that I don't have to.

[lurcher]

On 17/10/2022 16:17, v-chojas wrote: It is probably better to add the quoting only if there are special characters that need it. — Reply to this email directly, view it on GitHub <#113 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABYK62MVOO5AZBPDJM6GZJDWDVU2DANCNFSM53QTU35A>. You are receiving this because you were mentioned.Message ID: ***@***.***>

I have just checked in a simple addition that I think allows what is needed and adds extra flexibility. iusql can now take a single argument that starts with DSN=, DRIVER= or FILEDSN= and then you can pass whatever you want in. iusql -v "DSN={Name Of The DSN};PWD={Password With semicolons; and Spaces};UID={User Name}" It should pass that directly to SQLDriverConnect so you can use whatever you want.

[hughmcmaster]

Thanks for checking in that commit, @lurcher.

I spent some time debugging this issue further, using MS SQL and the FreeTDS ODBC driver to replicate the original system.

From my testing, I learnt that any password containing a semicolon must be enclosed in braces and must have a closing semicolon appended. This applies whether using a single connection string (as per your commit) or the standard space-separated command-line arguments.

For example, the following are equivalent:

iusql myDSN user '{some;Password123};'
iusql 'DSN=myDSN;UID=user;PWD={some;Password123};'

With braces and trailing semicolon, iusql works correctly with the password reported in the original post.

Suggestions:

• Do some basic string parsing on the password. We could try to fix the password by adding braces and the closing semicolon. Or we could abort and warn that they need to use the format above. Whichever way we go, I think adding a trailing semicolon should be done on the iusql side, since extra semicolons don't seem to matter. Let me know if you want me to draft a patch.
• Update iusql documentation to mention the format for this case.

[lurcher]

On 06/12/2022 11:19, Hugh McMaster wrote: Thanks for checking in that commit, @lurcher <https://github.com/lurcher>. I spent some time debugging this issue further, using MS SQL and the FreeTDS ODBC driver to replicate the original system. From my testing, I learnt that any password containing a semicolon must be enclosed in braces and must have a closing semicolon appended. This applies whether using a single connection string (as per your commit) or the standard space-separated command-line arguments. For example, the following are equivalent: |iusql myDSN user "{some;Password123};"| |iusql "DSN=myDSN;UID=user;PWD={some;Password123};"| With braces and trailing semicolon, iusql works correctly with the password reported in the original post. Suggestions: * Do some basic string parsing on the password. We could try to fix the password by adding braces and the closing semicolon. Or we could abort and warn that they need to use the format above. Whichever way we go, I think adding a trailing semicolon should be done on the |iusql| side, since extra semicolons don't seem to matter. Let me know if you want me to draft a patch. * Update |iusql| documentation to mention the format for this case. — Reply to this email directly, view it on GitHub <#113 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABYK62I5XEO26VF3DB27HOTWL4ONXANCNFSM53QTU35A>. You are receiving this because you were mentioned.Message ID: ***@***.***>

Well, I am strongly against adding the semicolon or in anyway trying to second guess what the user wants to do, or even what seems to work in the case of two drivers. I would argue that the original problem has a solution. Updating the docs would be my suggestion.