From 2e013eff40a437e137b303047f08c1741daef224 Mon Sep 17 00:00:00 2001 From: Spencer McIntyre Date: Fri, 24 Apr 2026 14:13:57 -0400 Subject: [PATCH 1/4] Fix Tree#_open_andx bit-field assignment The cfddc48 follow-up retyped OpenAndxRequest::ParameterBlock's search_attributes and file_attributes from uint16 to the smb_file_attributes BitField, but Tree#_open_andx still assigns Integer literals to them. BinData rejects this with "undefined method 'each_pair' for an instance of Integer", which breaks every Win9x code path that opens a file or pipe (e.g. Tree#open_file, Tree#open_pipe, and Client#net_share_enum_all when SMB1 is in use against a non-NT server). Use BitField#read with the packed mask, which round-trips to the same wire bytes (0x0016 -> directory|system|hidden, 0x0020 -> archive) without going through BinData::Struct#do_assign. Add four #open_file specs covering the OPEN_ANDX dispatch path that exercise the request builder and FID/size handling against a stubbed response. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/ruby_smb/smb1/tree.rb | 7 +++- spec/lib/ruby_smb/smb1/tree_spec.rb | 61 +++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/lib/ruby_smb/smb1/tree.rb b/lib/ruby_smb/smb1/tree.rb index 0a4baf5d..94a9ca58 100644 --- a/lib/ruby_smb/smb1/tree.rb +++ b/lib/ruby_smb/smb1/tree.rb @@ -310,8 +310,11 @@ def _open_andx(filename:, disposition:, read: true, write: false) end request.parameter_block.access_mode = access - request.parameter_block.search_attributes = 0x0016 - request.parameter_block.file_attributes = write ? 0x0020 : 0x0000 + # search_attributes / file_attributes are SMB_FILE_ATTRIBUTES BitField + # records, not plain uint16s — assign through #read to avoid BinData's + # each_pair-on-Integer NoMethodError when given a literal mask. + request.parameter_block.search_attributes.read([0x0016].pack('v')) + request.parameter_block.file_attributes.read([(write ? 0x0020 : 0x0000)].pack('v')) request.parameter_block.open_mode = nt_disposition_to_open_mode(disposition) fname = filename.dup diff --git a/spec/lib/ruby_smb/smb1/tree_spec.rb b/spec/lib/ruby_smb/smb1/tree_spec.rb index ea2c4bcc..02f7ad53 100644 --- a/spec/lib/ruby_smb/smb1/tree_spec.rb +++ b/spec/lib/ruby_smb/smb1/tree_spec.rb @@ -585,6 +585,67 @@ def build_find_first2_raw(blob, status: 0) end end + describe '#open_file (SMB_COM_OPEN_ANDX fallback)' do + # Win9x and other LAN-Manager-era servers don't advertise the NT SMBs + # capability, so #_open dispatches to #_open_andx instead of NT_CREATE_ANDX. + let(:open_andx_response) do + packet = RubySMB::SMB1::Packet::OpenAndxResponse.new + packet.smb_header.nt_status = 0 + packet.parameter_block.fid = 0x4242 + packet.parameter_block.file_data_size = 1234 + packet.parameter_block.resource_type = RubySMB::SMB1::ResourceType::DISK + packet + end + + before :example do + client.supports_nt_smbs = false + end + + it 'builds the OPEN_ANDX request without raising NoMethodError on bit-field assignment' do + allow(client).to receive(:send_recv).and_return(open_andx_response.to_binary_s) + expect { tree.open_file(filename: 'HELLO.TXT') }.not_to raise_error + end + + it 'serializes search_attributes / file_attributes as SMB_FILE_ATTRIBUTES bit-fields' do + sent = nil + allow(client).to receive(:send_recv) do |req| + sent = req + open_andx_response.to_binary_s + end + tree.open_file(filename: 'HELLO.TXT') + + # 0x0016 = directory | system | hidden in the SMB_FILE_ATTRIBUTES search half. + expect(sent.parameter_block.search_attributes.directory).to eq 1 + expect(sent.parameter_block.search_attributes.system).to eq 1 + expect(sent.parameter_block.search_attributes.hidden).to eq 1 + expect(sent.parameter_block.search_attributes.to_binary_s).to eq([0x0016].pack('v')) + + # Read-only open: file_attributes mask is zeroed. + expect(sent.parameter_block.file_attributes.to_binary_s).to eq([0x0000].pack('v')) + end + + it 'sets the SMB_FILE_ATTRIBUTE_ARCHIVE bit when opened for write' do + allow(client).to receive(:send_recv).and_return(open_andx_response.to_binary_s) + sent = nil + allow(client).to receive(:send_recv) do |req| + sent = req + open_andx_response.to_binary_s + end + tree.open_file(filename: 'HELLO.TXT', write: true) + + expect(sent.parameter_block.file_attributes.to_binary_s).to eq([0x0020].pack('v')) + expect(sent.parameter_block.file_attributes.archive).to eq 1 + end + + it 'returns a File handle whose FID and size come from the OPEN_ANDX response' do + allow(client).to receive(:send_recv).and_return(open_andx_response.to_binary_s) + file = tree.open_file(filename: 'HELLO.TXT') + expect(file).to be_a(RubySMB::SMB1::File) + expect(file.fid).to eq 0x4242 + expect(file.size).to eq 1234 + end + end + describe '#open_pipe' do let(:opts) { { filename: 'test', write: true } } before :example do From e422e10555c9dca59458f1bd6f1c671dff282baa Mon Sep 17 00:00:00 2001 From: Spencer McIntyre Date: Fri, 24 Apr 2026 15:03:18 -0400 Subject: [PATCH 2/4] Honor server-reported trans2 offsets in Tree#list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Win9x-era SMB1 servers (observed on Windows ME) pack trans2_parameters directly after byte_count with no 4-byte-alignment pad, and signal this by sending word_count=10 and a parameter_offset that points at the byte immediately following byte_count. BinData's Trans2::DataBlock#pad1_length unconditionally inserts the NT-style pad on read, so trans2_parameters and trans2_data both arrive shifted by the pad width — eos/sid come back garbled and the buffer is truncated, leaving #results with 0 entries. Detect the mismatch by comparing parameter_block.data_count to the bytes BinData actually read into trans2_data.buffer. When they differ and the raw response has enough bytes at the server-reported parameter_offset / data_offset, re-slice both sections from the raw response and hand them to the list loop — the effective trans2_parameters drive the eos / sid termination condition, and the trans2_data bytes feed #results through its new buffer: kwarg. Follows the same shape as the e243f02 fix for Rap::NetShareEnum. Regression test synthesizes a word_count=10, pad1=0 FIND_FIRST2 response carrying three SMB_INFO_STANDARD entries (".", "..", "FLAG.TXT.txt") matching the bytes captured from a live Windows ME target; before the fix BinData's buffer is 2 bytes short and 0 entries parse, after the fix all three round-trip. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../packet/trans2/find_first2_response.rb | 9 +++- lib/ruby_smb/smb1/tree.rb | 44 +++++++++++++++++-- spec/lib/ruby_smb/smb1/tree_spec.rb | 37 ++++++++++++++++ 3 files changed, 84 insertions(+), 6 deletions(-) diff --git a/lib/ruby_smb/smb1/packet/trans2/find_first2_response.rb b/lib/ruby_smb/smb1/packet/trans2/find_first2_response.rb index 03c7f519..9874269a 100644 --- a/lib/ruby_smb/smb1/packet/trans2/find_first2_response.rb +++ b/lib/ruby_smb/smb1/packet/trans2/find_first2_response.rb @@ -68,10 +68,15 @@ def initialize_instance # pad byte between entries (see MS-CIFS Appendix A, note <153>). # # @param klass [Class] the FileInformationClass class to read the data as + # @param buffer [String, nil] raw trans2_data bytes to parse instead of + # the BinData-parsed buffer. Used by callers that detect a padding + # mismatch between BinData's expected layout and what a Win9x-era + # server actually sent (no 4-byte alignment pad before the data), + # and want to re-feed the bytes from the server-reported data_offset. # @return [array] An array of structs holding the requested information # @raise [RubySMB::Error::InvalidPacket] if the string buffer is not a valid File Information packet - def results(klass, unicode:) - blob = data_block.trans2_data.buffer.to_binary_s.dup + def results(klass, unicode:, buffer: nil) + blob = (buffer || data_block.trans2_data.buffer.to_binary_s).dup if klass.new.respond_to?(:next_offset) read_next_offset_entries(klass, blob, unicode: unicode) else diff --git a/lib/ruby_smb/smb1/tree.rb b/lib/ruby_smb/smb1/tree.rb index 94a9ca58..ea64ac71 100644 --- a/lib/ruby_smb/smb1/tree.rb +++ b/lib/ruby_smb/smb1/tree.rb @@ -144,10 +144,16 @@ def list(directory: '\\', pattern: '*', unicode: true, raise RubySMB::Error::UnexpectedStatusCode, response.status_code end - results = response.results(type, unicode: unicode) - - eos = response.data_block.trans2_parameters.eos - sid = response.data_block.trans2_parameters.sid + t2p_override, t2d_override = win9x_trans2_overrides(response, raw_response) + results = if t2d_override + response.results(type, unicode: unicode, buffer: t2d_override) + else + response.results(type, unicode: unicode) + end + + effective_params = t2p_override || response.data_block.trans2_parameters + eos = effective_params.eos + sid = effective_params.sid last = results.last&.file_name while eos.zero? && last @@ -369,6 +375,36 @@ def build_open_andx_handle(filename, response) file end + # Win9x-era servers pack trans2_parameters right after byte_count with + # no 4-byte-alignment pad, but BinData's Trans2::DataBlock#pad1_length + # always inserts one for an NT-style response. When the server did not + # emit that pad, BinData reads both trans2_parameters and trans2_data + # shifted by the pad width, so `eos`, `sid`, and every entry in the + # buffer come back garbled. + # + # Detect the mismatch by comparing the declared data_count to what + # BinData actually read for the buffer, and when they differ re-slice + # both sections from the server-reported offsets in the raw response. + # Returns [trans2_parameters_record, trans2_data_bytes] — both values + # are nil when BinData's layout matched the wire (NT servers). + # Same shape as the fix applied to Rap::NetShareEnum in e243f02. + def win9x_trans2_overrides(response, raw_response) + declared = response.parameter_block.data_count.to_i + parsed = response.data_block.trans2_data.buffer.to_binary_s.bytesize + return [nil, nil] if declared == 0 || parsed == declared + + param_offset = response.parameter_block.parameter_offset.to_i + param_count = response.parameter_block.parameter_count.to_i + data_offset = response.parameter_block.data_offset.to_i + return [nil, nil] if raw_response.bytesize < data_offset + declared + return [nil, nil] if raw_response.bytesize < param_offset + param_count + + params_bytes = raw_response.byteslice(param_offset, param_count) + params = RubySMB::SMB1::Packet::Trans2::FindFirst2ResponseTrans2Parameters.read(params_bytes) + data_bytes = raw_response.byteslice(data_offset, declared) + [params, data_bytes] + end + # Sets ParameterBlock options for FIND_FIRST2 and # FIND_NEXT2 requests. In particular we need to do this # to tell the server to ignore the Trans2DataBlock as we are diff --git a/spec/lib/ruby_smb/smb1/tree_spec.rb b/spec/lib/ruby_smb/smb1/tree_spec.rb index 02f7ad53..279e9c8c 100644 --- a/spec/lib/ruby_smb/smb1/tree_spec.rb +++ b/spec/lib/ruby_smb/smb1/tree_spec.rb @@ -542,6 +542,43 @@ def build_find_first2_raw(blob, status: 0) allow(client).to receive(:send_recv).and_return(build_find_first2_raw('')) expect(tree.list(type: info_standard)).to eq([]) end + + context 'against a Win9x-era server that omits the trans2 4-byte alignment pad' do + # Wire layout: word_count=10 (no setup section), parameter_offset=55 + # points right after byte_count (no pad1), data_offset=66 points after + # trans2_parameters + 1-byte pad2. BinData's Trans2::DataBlock inserts + # its usual pad1 on read, so trans2_data.buffer arrives (data_count - + # pad1_length) bytes short and #results would otherwise see 0 entries. + # The Tree#list workaround detects the mismatch and re-slices the + # buffer from the server-reported data_offset. + def build_win9x_find_first2_raw + data_count = 87 + parameter_offset = 55 + data_offset = 66 + smb_header = "\xffSMB\x32".b + "\x00".b * 4 + "\x98".b + "\x03\x60".b + ("\x00".b * 20) + param_block = [10, data_count, 0, 10, parameter_offset, 0, + data_count, data_offset, 0, 0].pack('v*') + trans2_params = [0x0300, 3, 1, 0, 74].pack('v*') + entry1 = "\x98\x5c\x38\x70\x98\x5c\x00\x00\x98\x5c\x39\x70" \ + "\x00\x00\x00\x00\x00\x00\x00\x00\x10\x00\x01".b + '.' + entry2 = "\x98\x5c\x38\x70\x98\x5c\x00\x00\x98\x5c\x39\x70" \ + "\x00\x00\x00\x00\x00\x00\x00\x00\x10\x00\x02".b + '..' + entry3 = "\x98\x5c\x40\x70\x98\x5c\x00\x00\x98\x5c\x4c\x70" \ + "\x16\x00\x00\x00\x16\x00\x00\x00\x20\x00\x0c".b + 'FLAG.TXT.txt' + trans2_data = entry1 + "\x00".b + entry2 + "\x00".b + entry3 + "\x00".b + raise "data_count mismatch" unless trans2_data.bytesize == data_count + byte_count_value = 10 + 1 + data_count # 0 pad1 + params + 1 pad2 + data + smb_header + [10].pack('C') + param_block + + [byte_count_value].pack('v') + trans2_params + "\x00".b + trans2_data + end + + it 'parses the entries that BinData would otherwise drop due to missing pad1' do + allow(client).to receive(:send_recv).and_return(build_win9x_find_first2_raw) + results = tree.list(type: info_standard) + expect(results.map { |r| r.file_name.to_s }).to eq(['.', '..', 'FLAG.TXT.txt']) + expect(results.last.data_size).to eq 22 + end + end end end From b0cb9c76f3edbd44e521963628abb1b19495a477 Mon Sep 17 00:00:00 2001 From: Spencer McIntyre Date: Fri, 24 Apr 2026 15:38:43 -0400 Subject: [PATCH 3/4] Stringify RAP net_share_enum type to match SRVSVC Rap::NetShareEnum#net_share_enum returned :type as the raw Integer from share_info_1.shi1_type (e.g. 0x0003), while the SRVSVC equivalent in Dcerpc::Srvsvc#net_share_enum_all returned it as a pipe-joined string (e.g. "IPC|SPECIAL"). Consumers that want to enumerate shares without caring which transport carried the answer had to special-case the two APIs. Format the RAP type the same way: look up the low-byte base code against a local SHARE_TYPES table (MS-RAP 2.5.14 only defines DISK / PRINTER / DEVICE / IPC, so it's smaller than the SRVSVC variant), then append SPECIAL / TEMPORARY modifiers when those bits are set. Unknown base codes render as UNKNOWN(0xXXXX) so they round-trip visibly rather than getting silently dropped. Note: RAP share_type is 16 bits, so STYPE_SPECIAL and STYPE_TEMPORARY are 0x8000 / 0x4000 (not the 32-bit 0x80000000 / 0x40000000 values SRVSVC uses). Observed Windows ME responses do not set either bit, but other servers may. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/ruby_smb/rap/net_share_enum.rb | 25 ++++++++++++- spec/lib/ruby_smb/rap/net_share_enum_spec.rb | 38 ++++++++++++++++++-- 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/lib/ruby_smb/rap/net_share_enum.rb b/lib/ruby_smb/rap/net_share_enum.rb index 4b7120fc..1aa9bceb 100644 --- a/lib/ruby_smb/rap/net_share_enum.rb +++ b/lib/ruby_smb/rap/net_share_enum.rb @@ -19,6 +19,18 @@ module NetShareEnum # Default server receive-buffer size. DEFAULT_RECEIVE_BUFFER_SIZE = 0x1000 + # Share type codes carried in the low bits of `share_info_1.shi1_type` + # per MS-RAP 2.5.14. The RAP field is 16 bits wide, unlike the 32-bit + # SRVSVC variant in {RubySMB::Dcerpc::Srvsvc::SHARE_TYPES}. + SHARE_TYPES = { + 0x0000 => 'DISK', + 0x0001 => 'PRINTER', + 0x0002 => 'DEVICE', + 0x0003 => 'IPC' + }.freeze + STYPE_SPECIAL = 0x8000 + STYPE_TEMPORARY = 0x4000 + # Single share entry (`share_info_1`) as it appears on the wire. # MS-RAP 2.5.21. Fixed 20-byte layout. class ShareInfo1 < BinData::Record @@ -134,10 +146,21 @@ def parse_net_share_enum_response(response, raw_response) entry = ShareInfo1.read(data_bytes[offset, ShareInfo1.new.num_bytes]) { name: entry.netname.to_s.delete("\x00"), - type: entry.share_type + type: format_share_type(entry.share_type.to_i) } end.compact end + + # Format a RAP `share_info_1.shi1_type` value as a pipe-joined string in + # the same style as {RubySMB::Dcerpc::Srvsvc#net_share_enum_all}, so + # callers can consume both APIs uniformly. + def format_share_type(share_type) + base_bits = share_type & ~(STYPE_SPECIAL | STYPE_TEMPORARY) + parts = [SHARE_TYPES[base_bits] || format('UNKNOWN(0x%04x)', base_bits)] + parts << 'SPECIAL' unless (share_type & STYPE_SPECIAL).zero? + parts << 'TEMPORARY' unless (share_type & STYPE_TEMPORARY).zero? + parts.join('|') + end end end end diff --git a/spec/lib/ruby_smb/rap/net_share_enum_spec.rb b/spec/lib/ruby_smb/rap/net_share_enum_spec.rb index 5e00ddf3..357cfb21 100644 --- a/spec/lib/ruby_smb/rap/net_share_enum_spec.rb +++ b/spec/lib/ruby_smb/rap/net_share_enum_spec.rb @@ -47,11 +47,43 @@ def build_rap_response(status:, entries: []) ] allow(client).to receive(:send_recv).and_return(build_rap_response(status: 0, entries: entries)) expect(pipe.net_share_enum).to eq([ - { name: 'IPC$', type: 0x0003 }, - { name: 'DATA', type: 0x0000 } + { name: 'IPC$', type: 'IPC' }, + { name: 'DATA', type: 'DISK' } ]) end + it 'stringifies all MS-RAP 2.5.14 base share types' do + entries = [ + { name: 'DSK', type: 0x0000 }, + { name: 'PRN', type: 0x0001 }, + { name: 'DEV', type: 0x0002 }, + { name: 'IPC$', type: 0x0003 } + ] + allow(client).to receive(:send_recv).and_return(build_rap_response(status: 0, entries: entries)) + expect(pipe.net_share_enum.map { |s| s[:type] }).to eq(%w[DISK PRINTER DEVICE IPC]) + end + + it 'appends SPECIAL / TEMPORARY modifiers to the base type string' do + entries = [ + { name: 'HIDDEN$', type: 0x0000 | RubySMB::Rap::NetShareEnum::STYPE_SPECIAL }, + { name: 'TMP', type: 0x0000 | RubySMB::Rap::NetShareEnum::STYPE_TEMPORARY }, + { name: 'IPCH$', type: 0x0003 | RubySMB::Rap::NetShareEnum::STYPE_SPECIAL | + RubySMB::Rap::NetShareEnum::STYPE_TEMPORARY } + ] + allow(client).to receive(:send_recv).and_return(build_rap_response(status: 0, entries: entries)) + expect(pipe.net_share_enum.map { |s| s[:type] }).to eq([ + 'DISK|SPECIAL', + 'DISK|TEMPORARY', + 'IPC|SPECIAL|TEMPORARY' + ]) + end + + it 'formats an unknown base type code as UNKNOWN(0xXXXX)' do + entries = [{ name: 'Q', type: 0x0007 }] + allow(client).to receive(:send_recv).and_return(build_rap_response(status: 0, entries: entries)) + expect(pipe.net_share_enum.first[:type]).to eq('UNKNOWN(0x0007)') + end + it 'sends a Trans request targeting \\PIPE\\LANMAN with the tree id' do allow(client).to receive(:send_recv) do |request| expect(request).to be_a(RubySMB::SMB1::Packet::Trans::Request) @@ -136,7 +168,7 @@ def build_rap_response(status:, entries: []) shares = pipe.net_share_enum expect(shares.length).to eq(1) expect(shares[0][:name]).to eq('ABCDEFGHIJKL') - expect(shares[0][:type]).to eq(0) + expect(shares[0][:type]).to eq('DISK') end end From 32ceb8b5ca16c42ad563fcd3ae3f350a861ebf91 Mon Sep 17 00:00:00 2001 From: Spencer McIntyre Date: Fri, 24 Apr 2026 16:14:04 -0400 Subject: [PATCH 4/4] Extract Trans2 Win9x framing workaround into a mixin The server-reported-offset re-slice introduced in e422e10 lived inline on Tree#list, hardcoded to FindFirst2ResponseTrans2Parameters and only reachable through that one call site. Other Trans2 subcommands (OPEN2, QUERY_FILE_INFORMATION, QUERY_FS_INFORMATION, etc.) will need the same workaround if Win9x compatibility gets widened beyond directory listing, and duplicating the same four-line detect-and-slice block at each caller is exactly the kind of drift that makes Win9x quirks painful to chase later. Move the logic to RubySMB::SMB1::Packet::Trans2::Win9xFraming, a tiny mixin with a single #win9x_trans2_overrides(raw_response) method. The mixin consumes only fields that every Trans2 response already has (parameter_block.{parameter_offset, parameter_count, data_offset, data_count}; data_block.{trans2_parameters, trans2_data.buffer}), and the parameter-struct type is discovered from data_block.trans2_parameters.class so the same method works across subcommands without per-caller hardcoding. FindFirst2Response includes the mixin; Tree#list now calls response.win9x_trans2_overrides(raw_response) instead of the private helper it carried before. The private Tree#win9x_trans2_overrides is gone. New spec covers the mixin directly (NT-style no-op, zero data_count, Win9x-era slice for both parameters and data, truncated raw response) using FindFirst2Response as the concrete host so the field layouts stay real. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/ruby_smb/smb1/packet/trans2.rb | 1 + .../packet/trans2/find_first2_response.rb | 2 + .../smb1/packet/trans2/win9x_framing.rb | 68 +++++++++++ lib/ruby_smb/smb1/tree.rb | 32 +---- .../smb1/packet/trans2/win9x_framing_spec.rb | 113 ++++++++++++++++++ 5 files changed, 185 insertions(+), 31 deletions(-) create mode 100644 lib/ruby_smb/smb1/packet/trans2/win9x_framing.rb create mode 100644 spec/lib/ruby_smb/smb1/packet/trans2/win9x_framing_spec.rb diff --git a/lib/ruby_smb/smb1/packet/trans2.rb b/lib/ruby_smb/smb1/packet/trans2.rb index 09722775..728219ba 100644 --- a/lib/ruby_smb/smb1/packet/trans2.rb +++ b/lib/ruby_smb/smb1/packet/trans2.rb @@ -8,6 +8,7 @@ module Trans2 require 'ruby_smb/smb1/packet/trans2/query_information_level' require 'ruby_smb/smb1/packet/trans2/query_fs_information_level' require 'ruby_smb/smb1/packet/trans2/data_block' + require 'ruby_smb/smb1/packet/trans2/win9x_framing' require 'ruby_smb/smb1/packet/trans2/subcommands' require 'ruby_smb/smb1/packet/trans2/request' require 'ruby_smb/smb1/packet/trans2/request_secondary' diff --git a/lib/ruby_smb/smb1/packet/trans2/find_first2_response.rb b/lib/ruby_smb/smb1/packet/trans2/find_first2_response.rb index 9874269a..b1889a1a 100644 --- a/lib/ruby_smb/smb1/packet/trans2/find_first2_response.rb +++ b/lib/ruby_smb/smb1/packet/trans2/find_first2_response.rb @@ -41,6 +41,8 @@ class FindFirst2ResponseDataBlock < RubySMB::SMB1::Packet::Trans2::DataBlock # This class represents an SMB1 Trans2 FIND_FIRST2 Response Packet as defined in # [2.2.6.2.2 Response](https://msdn.microsoft.com/en-us/library/ee441704.aspx) class FindFirst2Response < RubySMB::GenericPacket + include RubySMB::SMB1::Packet::Trans2::Win9xFraming + COMMAND = RubySMB::SMB1::Commands::SMB_COM_TRANSACTION2 class ParameterBlock < RubySMB::SMB1::Packet::Trans2::Response::ParameterBlock diff --git a/lib/ruby_smb/smb1/packet/trans2/win9x_framing.rb b/lib/ruby_smb/smb1/packet/trans2/win9x_framing.rb new file mode 100644 index 00000000..c11f73e0 --- /dev/null +++ b/lib/ruby_smb/smb1/packet/trans2/win9x_framing.rb @@ -0,0 +1,68 @@ +module RubySMB + module SMB1 + module Packet + module Trans2 + # Shared workaround for pre-NT / LAN Manager-era servers (observed on + # Windows 9x / ME) that pack `trans2_parameters` directly after + # `byte_count` with no 4-byte-alignment pad, and `trans2_data` with + # whatever padding they feel like — always smaller than the NT-style + # alignment BinData unconditionally assumes via {DataBlock#pad1_length} + # and {DataBlock#pad2_length}. When that happens both sections land in + # the wrong place and `eos`, `sid`, `last_name_offset`, and every + # entry in the data buffer come back garbled. + # + # Fixing this in BinData itself (by having pad1/pad2 consult + # `parameter_block.parameter_offset` / `data_offset`) is the natural + # design, but cross-field lookups during field-read callbacks corrupt + # BinData's registered-class resolution cache, causing unrelated + # Trans2 responses to round-trip their `parameter_block` / `data_block` + # through the base classes instead of the concrete subclasses. So + # instead we surface the raw response bytes at the call site and let + # the response slice both sections from the offsets the server + # reported in its `parameter_block`. + # + # Mix into any {RubySMB::SMB1::Packet::Trans2} response whose caller + # holds on to the raw response bytes. The response itself must have + # the standard {Trans2::Response::ParameterBlock} shape + # (`parameter_offset` / `parameter_count` / `data_offset` / + # `data_count`) and a `data_block` with `trans2_parameters` and + # `trans2_data.buffer` fields — every concrete Trans2 response does. + # + # Same slicing pattern as {RubySMB::Rap::NetShareEnum#parse_net_share_enum_response} + # uses for the sibling Trans (not Trans2) response type. + module Win9xFraming + # Returns `[effective_trans2_parameters, effective_trans2_data_bytes]` + # when the server's layout differs from BinData's, or `[nil, nil]` + # when BinData already read the full buffer (standard NT-era servers). + # + # When a non-nil pair is returned, callers should prefer the override + # values over the BinData-parsed ones: + # + # params_ovr, data_ovr = response.win9x_trans2_overrides(raw) + # params = params_ovr || response.data_block.trans2_parameters + # data = data_ovr || response.data_block.trans2_data.buffer.to_binary_s + # + # @param raw_response [String] the raw bytes the response was read from. + # @return [Array(BinData::Record, String), Array(nil, nil)] + def win9x_trans2_overrides(raw_response) + declared_data = parameter_block.data_count.to_i + parsed_data = data_block.trans2_data.buffer.to_binary_s.bytesize + return [nil, nil] if declared_data.zero? || parsed_data == declared_data + + param_offset = parameter_block.parameter_offset.to_i + param_count = parameter_block.parameter_count.to_i + data_offset = parameter_block.data_offset.to_i + return [nil, nil] if raw_response.bytesize < data_offset + declared_data + return [nil, nil] if raw_response.bytesize < param_offset + param_count + + params_bytes = raw_response.byteslice(param_offset, param_count) + params_class = data_block.trans2_parameters.class + params = params_class.read(params_bytes) + data_bytes = raw_response.byteslice(data_offset, declared_data) + [params, data_bytes] + end + end + end + end + end +end diff --git a/lib/ruby_smb/smb1/tree.rb b/lib/ruby_smb/smb1/tree.rb index ea64ac71..030613f3 100644 --- a/lib/ruby_smb/smb1/tree.rb +++ b/lib/ruby_smb/smb1/tree.rb @@ -144,7 +144,7 @@ def list(directory: '\\', pattern: '*', unicode: true, raise RubySMB::Error::UnexpectedStatusCode, response.status_code end - t2p_override, t2d_override = win9x_trans2_overrides(response, raw_response) + t2p_override, t2d_override = response.win9x_trans2_overrides(raw_response) results = if t2d_override response.results(type, unicode: unicode, buffer: t2d_override) else @@ -375,36 +375,6 @@ def build_open_andx_handle(filename, response) file end - # Win9x-era servers pack trans2_parameters right after byte_count with - # no 4-byte-alignment pad, but BinData's Trans2::DataBlock#pad1_length - # always inserts one for an NT-style response. When the server did not - # emit that pad, BinData reads both trans2_parameters and trans2_data - # shifted by the pad width, so `eos`, `sid`, and every entry in the - # buffer come back garbled. - # - # Detect the mismatch by comparing the declared data_count to what - # BinData actually read for the buffer, and when they differ re-slice - # both sections from the server-reported offsets in the raw response. - # Returns [trans2_parameters_record, trans2_data_bytes] — both values - # are nil when BinData's layout matched the wire (NT servers). - # Same shape as the fix applied to Rap::NetShareEnum in e243f02. - def win9x_trans2_overrides(response, raw_response) - declared = response.parameter_block.data_count.to_i - parsed = response.data_block.trans2_data.buffer.to_binary_s.bytesize - return [nil, nil] if declared == 0 || parsed == declared - - param_offset = response.parameter_block.parameter_offset.to_i - param_count = response.parameter_block.parameter_count.to_i - data_offset = response.parameter_block.data_offset.to_i - return [nil, nil] if raw_response.bytesize < data_offset + declared - return [nil, nil] if raw_response.bytesize < param_offset + param_count - - params_bytes = raw_response.byteslice(param_offset, param_count) - params = RubySMB::SMB1::Packet::Trans2::FindFirst2ResponseTrans2Parameters.read(params_bytes) - data_bytes = raw_response.byteslice(data_offset, declared) - [params, data_bytes] - end - # Sets ParameterBlock options for FIND_FIRST2 and # FIND_NEXT2 requests. In particular we need to do this # to tell the server to ignore the Trans2DataBlock as we are diff --git a/spec/lib/ruby_smb/smb1/packet/trans2/win9x_framing_spec.rb b/spec/lib/ruby_smb/smb1/packet/trans2/win9x_framing_spec.rb new file mode 100644 index 00000000..d5b414d2 --- /dev/null +++ b/spec/lib/ruby_smb/smb1/packet/trans2/win9x_framing_spec.rb @@ -0,0 +1,113 @@ +require 'spec_helper' + +RSpec.describe RubySMB::SMB1::Packet::Trans2::Win9xFraming do + # FindFirst2Response is the first production consumer of the mixin; its + # fixture data already covers every code path in #win9x_trans2_overrides + # (zero-length buffer, on-wire match, server-reported mismatch, truncated + # raw response). Using it here keeps the spec grounded in real field + # layouts without standing up an anonymous host class. + let(:info_std) do + RubySMB::SMB1::Packet::Trans2::FindInformationLevel::FindInfoStandard + end + + # Reusable fixtures: NT-style (with pad1=3) and Win9x-style (pad1=0) raw + # FindFirst2Response frames carrying the same single-entry payload so the + # overrides helper sees the same server-declared offsets differ from what + # BinData positionally reads. + let(:single_entry_bytes) do + "\x98\x5c\x38\x70\x98\x5c\x00\x00\x98\x5c\x39\x70".b + + "\x00\x00\x00\x00\x00\x00\x00\x00\x10\x00\x01".b + '.' + end + let(:data_count) { single_entry_bytes.bytesize } + + def smb_header + "\xffSMB\x32".b + "\x00".b * 4 + "\x98".b + "\x03\x60".b + ("\x00".b * 20) + end + + def build_response(parameter_offset:, data_offset:, word_count:, pad1: 0, pad2: 0) + # The concrete field layout of FindFirst2Response's parameter_block + # changes with word_count: 11 words include a 1-entry setup array; + # 10 words (Win9x style) omit it. trans2_parameters itself is a + # fixed 10-byte struct (sid, search_count, eos, ea_err_off, last_name_off). + trans2_params = [0x0300, 1, 1, 0, 0].pack('v*') + pb_values = [10, data_count, 0, 10, parameter_offset, 0, + data_count, data_offset, 0] + # word_count=11 → setup_count(1) + reserved2(0) + 1-word setup array + # word_count=10 → setup_count(0) + reserved2(0), no setup array + pb_tail = word_count == 11 ? "\x01\x00".b + [1].pack('v') : "\x00\x00".b + param_block = pb_values.pack('v*') + pb_tail + byte_count = pad1 + 10 + pad2 + data_count + smb_header + [word_count].pack('C') + param_block + + [byte_count].pack('v') + ("\x00".b * pad1) + trans2_params + + ("\x00".b * pad2) + single_entry_bytes + end + + describe '#win9x_trans2_overrides' do + context 'when BinData has already read the full buffer (NT-style server)' do + it 'returns [nil, nil]' do + # NT-era response: word_count=11 with 1-word setup, pad1=3, pad2=2 + # → trans2_parameters at offset 60, trans2_data at 72. BinData's + # Trans2::DataBlock positional read lands exactly on the wire data. + raw = build_response( + parameter_offset: 60, data_offset: 72, + word_count: 11, pad1: 3, pad2: 2 + ) + response = RubySMB::SMB1::Packet::Trans2::FindFirst2Response.read(raw) + expect(response.win9x_trans2_overrides(raw)).to eq([nil, nil]) + end + end + + context 'when the server declared no trans2_data (data_count == 0)' do + it 'returns [nil, nil]' do + raw = build_response( + parameter_offset: 60, data_offset: 72, + word_count: 11, pad1: 3, pad2: 2 + ) + response = RubySMB::SMB1::Packet::Trans2::FindFirst2Response.read(raw) + response.parameter_block.data_count = 0 + expect(response.win9x_trans2_overrides(raw)).to eq([nil, nil]) + end + end + + context 'when the server used Win9x-era framing (no pad1)' do + it 'returns trans2_parameters re-read at the server-reported offset' do + raw = build_response( + parameter_offset: 55, data_offset: 66, + word_count: 10, pad1: 0, pad2: 1 + ) + response = RubySMB::SMB1::Packet::Trans2::FindFirst2Response.read(raw) + params, = response.win9x_trans2_overrides(raw) + expect(params).to be_a(RubySMB::SMB1::Packet::Trans2::FindFirst2ResponseTrans2Parameters) + expect(params.sid).to eq 0x0300 + expect(params.search_count).to eq 1 + expect(params.eos).to eq 1 + end + + it 'returns the trans2_data bytes sliced from the server-reported offset' do + raw = build_response( + parameter_offset: 55, data_offset: 66, + word_count: 10, pad1: 0, pad2: 1 + ) + response = RubySMB::SMB1::Packet::Trans2::FindFirst2Response.read(raw) + _, data_bytes = response.win9x_trans2_overrides(raw) + expect(data_bytes).to eq single_entry_bytes + # And #results can read entries from it through the buffer: kwarg. + entries = response.results(info_std, unicode: false, buffer: data_bytes) + expect(entries.length).to eq 1 + expect(entries.first.file_name.to_s).to eq '.' + end + end + + context 'when the raw response is truncated before the server-reported offsets' do + it 'returns [nil, nil] rather than raising' do + raw = build_response( + parameter_offset: 55, data_offset: 66, + word_count: 10, pad1: 0, pad2: 1 + ) + response = RubySMB::SMB1::Packet::Trans2::FindFirst2Response.read(raw) + truncated = raw.byteslice(0, raw.bytesize - 20) + expect(response.win9x_trans2_overrides(truncated)).to eq([nil, nil]) + end + end + end +end