-
Notifications
You must be signed in to change notification settings - Fork 13.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add support for boolean datatypes on mssql #18872
Conversation
6dfdcc5
to
c0626d1
Compare
Just to help with testing - would you mind sharing those queries, or sharing the SQL for creating new tables and values that replicate this issue 👍 |
I'll edit the steps now! |
lib/rex/proto/mssql/client_mixin.rb
Outdated
@@ -178,6 +178,13 @@ def mssql_parse_tds_reply(data, info) | |||
col[:id] = :int | |||
col[:int_size] = data.slice!(0, 1).unpack('C')[0] | |||
|
|||
when 50 | |||
col[:id] = :sybbit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where does sybbit
come from? 👀
Is that this?
BITTYPE = %x32 ; Bit
https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-tds/859eb3d2-80d3-40f6-a637-414552c9c552
BITNTYPE = %x68
https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-tds/ce3183a6-9d89-47e8-a02f-de5a1a1303de
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, is the existing implementation and terminology from freetds rather than the microsoft tds spec?
0x32 50 SYBBIT no 0
...
0x68 104 SYBBITN yes 1
https://www.freetds.org/tds.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got bitntype from the microsoft tds spec and sybbit from the freetds spec - I'm open to feedback on the names. I think it would be reasonable to change to both of the freetds names, or :bit and :nullable_bit to be more specific?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah; From what I understand, in freetds - syb
is just a prefix that C developers use, and it comes from the sybase
product as far as I can see:
TDS Version | Supported Products |
---|---|
4.2 | Sybase SQL Server < 10 and Microsoft SQL Server 6.5 |
5.0 | Sybase SQL Server >= 10 |
7.0 | Microsoft SQL Server 7.0 |
7.1 | Microsoft SQL Server 2000 |
7.2 | Microsoft SQL Server 2005 |
So the naming convention should just probably be:
SYBBIT => bit
SYBBITN => bitn
lib/rex/proto/mssql/client_mixin.rb
Outdated
when :bitntype | ||
has_value = data.slice!(0, 1).unpack("C")[0] | ||
if has_value == 0 | ||
row << 'NULL' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be row << nil
? 👀
The rationale being, the string 'NULL'
is definitely not the same as an actual Ruby nil
If you want it to appear as NULL
in the table outputs of modules, you'll have to handle that in your presentation layer - not here 👍
lib/rex/proto/mssql/client_mixin.rb
Outdated
col[:id] = :sybbit | ||
|
||
when 104 | ||
col[:id] = :bitntype |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
col[:id] = :bitntype | |
col[:id] = :bitn |
lib/rex/proto/mssql/client_mixin.rb
Outdated
@@ -178,6 +178,13 @@ def mssql_parse_tds_reply(data, info) | |||
col[:id] = :int | |||
col[:int_size] = data.slice!(0, 1).unpack('C')[0] | |||
|
|||
when 50 | |||
col[:id] = :sybbit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
col[:id] = :sybbit | |
col[:id] = :bit |
lib/rex/proto/mssql/client_mixin.rb
Outdated
row << data.slice!(0, 1).unpack("C")[0] | ||
end | ||
|
||
when :sybbit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when :sybbit | |
when :bit |
lib/rex/proto/mssql/client_mixin.rb
Outdated
@@ -292,6 +299,17 @@ def mssql_parse_tds_row(data, info) | |||
when :tinyint | |||
row << data.slice!(0, 1).unpack("C")[0] | |||
|
|||
when :bitntype |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when :bitntype | |
when :bitn |
c0626d1
to
55fab27
Compare
55fab27
to
854dcc5
Compare
Release NotesUpdates the MSSQL modules to support querying database rows that contain boolean bit values |
In newer TDS Protocol versions, two different bit datatypes were added, one that allows null and one that doesn't, to allow for boolean support. To this point, our MSSQL code does not support these values, and causes "unknown column type" errors.
Verification
List the steps needed to make sure this thing works
mssql_query
toquery
inlib/rex/post/mssql/ui/console.rb
(PR incoming)msfconsole
use mssql_login
run CreateSession=true ...
to get a sessionsessions -i -1
Setting up MSSQL Table
in your mssql instance, run the following steps:
create table your_table (nullable_bit bit);
grant select on your_table to public;
alter table your_table add strict_bit bit not null default 0;
insert into your_table (nullable_bit, strict_bit) values (0, 0);
insert into your_table (nullable_bit, strict_bit) values (0, 1);
insert into your_table (nullable_bit, strict_bit) values (1, 0);
insert into your_table (nullable_bit, strict_bit) values (1, 1);
insert into your_table (nullable_bit, strict_bit) values (NULL, 0);
insert into your_table (nullable_bit, strict_bit) values (NULL, 1);
previously, running
select * from your_table;
would break this if run from your mssql client in metasploit framework. This now should look like:or from modules: