bpo-43284: Update platform.win32_ver to use platform._syscmd_ver instead of sys.getwindowsversion().platform_version for determining accurate Windows version

[shreyanavigyan]

platform.win32_ver derives the Windows version from sys.getwindowsversion().platform_version which in turn derives the version from kernel32.dll (which can be of a different version than Windows itself). Therefore this PR updates the platform.win32_ver to determine the version using the platform module's _syscmd_ver private function to return an accurate version.

More discussions are held at: https://bugs.python.org/issue43284

https://bugs.python.org/issue43284

[zooba]

I also noticed that _norm_version has a bug that is back in the code path with this change:

    try:
        ints = map(int, l)
    except ValueError:
        strings = l
    else:
        strings = list(map(str, ints))

The ValueError is never going to be raised there because map is lazy. We probably just want to merge the else block into the try: strings = list(map(str, map(int, l)))

[shreyanavigyan]

Are you suggesting to make the change in this PR itself?

[zooba]

Yes, please. (They don't exist, but any tests for these cases would be breaking on your PR because you've reintroduced the call to that function.)

[shreyanavigyan]

Change applied.

[shreyanavigyan]

I also noticed something that can cause naming conflicts later. In the function win32_ver the minor version is stored in a variable named min which is also the name of a built-in function. Therefore I propose to change the variable name from min to minor to avoid naming conflicts. I also think the maj variable should be changed to major.

[zooba]

If you'd like to make more changes, go ahead. Just let us know when it's ready.

[shreyanavigyan]

Ok

[shreyanavigyan]

PR is ready for reviewing and if applicable then merging. (No more commits will be made)

[miss-islington]

Thanks @shreyanavigyan for the PR, and @zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

[miss-islington]

Thanks @shreyanavigyan for the PR, and @zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

[bedevere-bot]

GH-25523 is a backport of this pull request to the 3.9 branch.

[bedevere-bot]

GH-25524 is a backport of this pull request to the 3.8 branch.

[szmcdull]

calling shell command may lead to security vulnerability. I'm strongly against this. I'm implementing a cpython wrapper that would disable methods like os.system, subprocess.Popen etc. But I found that many packages are calling platform.system().

IMO https://bugs.python.org/issue43284 is the expected behavior. The process is running in compatible mode, so an old version was reported. If you want the exact version, you should disable compatible mode.

[szmcdull]

In linux platform.system() also calls external command uname 😭

[eryksun]

calling shell command may lead to security vulnerability.

Running the ver command via subprocess.check_output() with shell=True executes the "%ComSpec%" shell. This should be the fully qualified path "%SystemRoot%\System32\cmd.exe". If %ComSpec% isn't a fully-qualified path, or if it's hijacked, then the system is already compromised.

Anyway, stating with Python 3.12 you'll probably be happy to discover that win32_ver() first tries a WMI query that's implemented by a new _wmi extension module. There are no plans to backport this extension module to previous versions, however.

The process is running in compatible mode, so an old version was reported. If you want the exact version, you should disable compatible mode.

The platform module is intended to return information about the machine and installed operating system, for logging and such. On Windows, scripts should use sys.getwindowsversion() to get the API version. The latter can be a compatibility version. For example, if the interpreter is embedded in an application that has no manifest, the reported OS version is Windows 8.

[szmcdull]

Running the ver command via subprocess.check_output() with shell=True executes the "%ComSpec%" shell. This should be the fully qualified path "%SystemRoot%\System32\cmd.exe". If %ComSpec% isn't a fully-qualified path, or if it's hijacked, then the system is already compromised.

Python should provide an option to disable external commands, just like PHP does. It's not only about hacking env variables, but also raises risks when string injection occurs. In my code I use C# to call python functions, which are user functions providing trading algorithms. But I don't want these user functions to call any external commands. Right now if I detect and prevent external command calls, packages like pandas/numpy would fail because they are calling platform.system(), which in turn calls external commands.