Skip to content

Windows installer: PATH is written to HKCU and as REG_SZ, even for an all-users install #6760

Description

@xyzzy-foo

Windows installer: PATH is written to HKCU and as REG_SZ, even for an all-users install

Summary

dist/windows/rizin.iss always adds {app}\bin to the per-user PATH
(HKCU\Environment) and writes it with ValueType: string, i.e. REG_SZ.

The installer supports both per-user and all-users installs
(PrivilegesRequired=lowest + PrivilegesRequiredOverridesAllowed=dialog), and the
file location already follows the selected mode via {autopf}. The registry entries
do not: they are hardcoded to HKCU. There is also no ChangesEnvironment=yes, so
running applications are never notified about the change.

Affects v0.9.1 and current dev (the script is identical in both).

Current code

dist/windows/rizin.iss:

[Registry]
Root: HKCU; Subkey: "Environment"; ValueType: string; ValueName: "Path"; ValueData: "{reg:HKCU\Environment,Path};{app}\bin"; Check: NeedsAddPath(ExpandConstant('{app}\bin'));
Root: HKCU; SubKey: "SOFTWARE\Classes\*\shell\rizin"; ValueType: string; ValueData: "Open in Rizin"; Flags: uninsdeletekey;
...

Problems

1. Scope mismatch (all-users install)

When the user picks "Install for all users" in the privileges dialog, Rizin is
installed to C:\Program Files\Rizin and registered under
HKLM\...\Uninstall, but PATH is modified only for the account that ran Setup.
Other users and service accounts on the machine do not see rizin on PATH.

If Setup is elevated with a different administrator account, the PATH entry ends
up in that administrator's profile instead of the user who is actually installing
Rizin.

PATH cannot be handled with a single Root: HKA entry, because the key name
differs per hive (SYSTEM\CurrentControlSet\Control\Session Manager\Environment vs.
Environment), so it needs two entries with Check: IsAdminInstallMode. The four
shell-extension entries below it, however, can simply use Root: HKA, since
SOFTWARE\Classes\*\shell\... is valid in both hives.

2. ValueType: string downgrades PATH to REG_SZ

ValueType: string writes REG_SZ unconditionally, replacing the existing value
type. From Projects/Src/Setup.Install.pas in jrsoftware/issrc:

rtString, rtExpandString, rtMultiString: begin
    NewType := REG_SZ;
    case Typ of
      rtExpandString: NewType := REG_EXPAND_SZ;
      ...
    if roPreserveStringType in Options then begin
      if (RegQueryValueEx(K, PChar(N), nil, @ExistingType, nil, nil) = ERROR_SUCCESS) and
         ((ExistingType = REG_SZ) or (ExistingType = REG_EXPAND_SZ)) then
        NewType := ExistingType;
    end;

Windows PATH is normally REG_EXPAND_SZ so that entries can reference other
variables. If a user PATH contains, for example, %ANDROID_HOME%\platform-tools,
rewriting the value as REG_SZ would make those references stop resolving, silently:
the %...% text is then taken literally.

This is only a type problem, not a data problem: {reg:...} reads the raw value via
RegQueryStringValue -> RegQueryValueEx (Projects/Src/Shared.CommonFunc.pas)
without expanding it, so existing %VAR% references survive the append as text.

The same issue would become more severe if problem 1 were fixed naively: the machine
PATH contains %SystemRoot% and similar by default, so writing it as REG_SZ
would break the system PATH outright.

Inno Setup offers two ways to avoid this: ValueType: expandsz, or
Flags: preservestringtype to keep whatever type the value already has.

3. No ChangesEnvironment=yes

Without this [Setup] directive (default no), Setup does not notify running
applications to reload their environment, so the new PATH is not visible to
already-running shells or Explorer-launched programs until re-login.

Steps to reproduce

  1. Run rizin_installer-v0.9.1-x86_64.exe and choose "Install for all users".
  2. Observe the install location C:\Program Files\Rizin and the uninstall entry
    under HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall\{...}_is1.
  3. Inspect the registry:
$u = [Microsoft.Win32.Registry]::CurrentUser.OpenSubKey('Environment')
$u.GetValueKind('Path')
$u.GetValue('Path', $null, 'DoNotExpandEnvironmentNames')      # contains C:\Program Files\Rizin\bin

The machine PATH is unchanged, and the entry is in the user PATH instead.

A note on scope: I hit problem 1 on a real install and verified it in the registry.
Problem 2 I did not verify before/after on a clean machine — it is what the
ValueType: string entry implies given the Inno Setup code quoted above, so please
confirm the value type yourself when testing a fix.

Suggested fix

[Setup]
ChangesEnvironment=yes

[Registry]
; PATH: needs one entry per hive, the key name differs
Root: HKLM; Subkey: "SYSTEM\CurrentControlSet\Control\Session Manager\Environment"; \
  ValueType: expandsz; ValueName: "Path"; ValueData: "{olddata};{app}\bin"; \
  Check: IsAdminInstallMode and NeedsAddPath(ExpandConstant('{app}\bin'))
Root: HKCU; Subkey: "Environment"; \
  ValueType: expandsz; ValueName: "Path"; ValueData: "{olddata};{app}\bin"; \
  Check: (not IsAdminInstallMode) and NeedsAddPath(ExpandConstant('{app}\bin'))

; shell extensions: HKA follows the install mode, key name is the same in both hives
Root: HKA; SubKey: "SOFTWARE\Classes\*\shell\rizin"; ValueType: string; ValueData: "Open in Rizin"; Flags: uninsdeletekey;
Root: HKA; SubKey: "SOFTWARE\Classes\*\shell\rizin\command"; ValueType: string; ValueData: "{app}\bin\rizin.exe %1"; Flags: uninsdeletekey;
Root: HKA; SubKey: "SOFTWARE\Classes\*\shell\rizind"; ValueType: string; ValueData: "Open in Rizin debugger"; Flags: uninsdeletekey;
Root: HKA; SubKey: "SOFTWARE\Classes\*\shell\rizind\command"; ValueType: string; ValueData: "{app}\bin\rizin.exe -d %1"; Flags: uninsdeletekey;

{olddata} replaces {reg:HKCU\Environment,Path} so the hive is not hardcoded a
second time, and NeedsAddPath() in [Code] needs the same branch, reading from
HKEY_LOCAL_MACHINE / SYSTEM\CurrentControlSet\Control\Session Manager\Environment
when IsAdminInstallMode is true.

Side note: PATH is left behind on uninstall

The PATH entry is never removed when Rizin is uninstalled. This one is not really
avoidable declaratively — uninsdeletevalue would delete the whole PATH value —
so it would need CurUninstallStepChanged in [Code] if you want to handle it.
Filing it here only for completeness; feel free to split it out.

Environment

  • Rizin v0.9.1 (x86_64 installer), Windows 11 26200
  • Also checked against dev: dist/windows/rizin.iss is unchanged

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions