Bug #20096: Ruby 3.2.2 win32/registry: Junk appended to Windows Registry String Value

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: ruby 3.2.2 (2023-03-30 revision e51014f9c0) [x64-mswin64_140]
• Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: DONE, 3.3: DONE

[jay4rubydev (Jay M)]

Ruby Version:
ruby 3.2.2 (2023-03-30 revision e51014f9c0) [x64-mswin64_140]
Compiler: MSVC 2019 - Microsoft (R) C/C++ Optimizing Compiler Version 19.29.30147 for x64.

Issue: win32/registry adds junk to Windows Registry String Value

Code:
require 'win32/registry'
win_oracle_key = "SOFTWARE\MicroSoft"
reg=Win32::Registry::HKEY_LOCAL_MACHINE.open(win_oracle_key, Win32::Registry::KEY_ALL_ACCESS)
inst_loc_key = "inst_loc"
inv_dir="C:\Program Files\Tester\ModuleInfo"
reg[inst_loc_key] = inv_dir

Result:
Registry contains:
C:\Program Files\Tester\ModuleInfo爀

Observation: Looks like memory overread

Expected Result - without the junk:
C:\Program Files\Tester\ModuleInfo

After changing the code in registry.rb:
From:
def write(name, type, data)

  termsize = 0
  case type
  when REG_SZ, REG_EXPAND_SZ
    data = data.encode(WCHAR)
    termsize = WCHAR_SIZE

To:

def write(name, type, data)

  termsize = 0
  case type
  when REG_SZ, REG_EXPAND_SZ
    enc_data = data.encode(WCHAR)
	# Add NULL WCHAR for string data and don't set the termsize because 
	# enc_data.bytesize will now include the size of the NULL character.
	enc_data += WCHAR_NUL
    # termsize = WCHAR_SIZE
  ...

[kjtsanaktsidis (KJ Tsanaktsidis)]

Thank you for your report. Your diagnosis looks right, this probably is a memory overread. Your patch looks reasonable - are you able to open this as a Github pull request?

Also, are you familiar with REG_MULTI_SZ? To my quick glance, that looks like it should also not need the termsize value (it's already appending WCHAR_NUL like your patch is). Do you think that should be removed too?

[jay4rubydev (Jay M)]

I agree about REG_MULTI_SZ  - from checking the documentation of https://learn.microsoft.com/en-us/windows/win32/api/winreg/nf-winreg-regsetvalueexw:
With the REG_MULTI_SZ data type, the string must be terminated with two null characters.

This is being done in registry.rb, so no need to add the termsize.

Sorry, I am not set up for git pull requests in my dev env.

[kjtsanaktsidis (KJ Tsanaktsidis)]

I agree about REG_MULTI_SZ - from checking the documentation of https://learn.microsoft.com/en-us/windows/win32/api/winreg/nf-winreg-regsetvalueexw:
With the REG_MULTI_SZ data type, the string must be terminated with two null characters.

This is being done in registry.rb, so no need to add the termsize.

OK, great, thank you.

Sorry, I am not set up for git pull requests in my dev env.

No worries, I opened https://github.com/ruby/ruby/pull/9381 for this.

[kjtsanaktsidis (KJ Tsanaktsidis)]

Fixed by merging https://github.com/ruby/ruby/pull/9381. I'll mark this to be backported to 3.2 as well.

[kjtsanaktsidis (KJ Tsanaktsidis)]

• Status changed from Open to Closed
• Backport changed from 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN to 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: REQUIRED, 3.3: REQUIRED

[kjtsanaktsidis (KJ Tsanaktsidis)]

Backport PR's:

• https://github.com/ruby/ruby/pull/9470
• https://github.com/ruby/ruby/pull/9471

[nagachika (Tomoyuki Chikanaga)]

• Backport changed from 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: REQUIRED, 3.3: REQUIRED to 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: DONE, 3.3: REQUIRED

ruby_3_2 5dae6eb55e9785c8329708e55a49a280a344cdc1 merged revision(s) 051a874325c177e040301878069c2b28f5d06ce6.

[naruse (Yui NARUSE)]

• Backport changed from 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: DONE, 3.3: REQUIRED to 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: DONE, 3.3: DONE

ruby_3_3 ade02f3c8909a8bf630af2c88f00b7bd7ff02682 merged revision(s) 051a874325c177e040301878069c2b28f5d06ce6.