diff options
author | Bernhard Urban-Forster <lewurm@gmail.com> | 2022-09-19 12:55:00 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-09-19 12:55:00 +0200 |
commit | 5d46b358d31ff7f0a67b104ad4cffffa032b3b35 (patch) | |
tree | 39b4b1307225933e068742a493a1a84420ad9cbb | |
parent | 9c38dd74e8a393371e6547b9404c303fa85a1459 (diff) | |
download | psutil-5d46b358d31ff7f0a67b104ad4cffffa032b3b35.tar.gz |
[macOS] Fix out-of-bounds read around sysctl_procargs (#2135)
The buffer size is initially determined by querying `argmax`:
https://github.com/giampaolo/psutil/blob/9f9a82d02c901f62512236b44edb050f84cbe5b6/psutil/arch/osx/process_info.c#L260-L265
This length is passed to the `sysctl()` call for querying the procargs. `sysctl()` is allowed to set it to a smaller value, to indicate the buffer wasn't fully used. However this is not passed back to the caller "owning" that memory, and also parsing the result.
Depending on the allocator, it's possible that we can observe "garbage" values, consider this example:
```python
import psutil
import subprocess
import os
import sys
environment_cookie = "__MAGIC_ENV_COOKIE__"
for i in range(0,200):
os.environ["PATH"] = os.environ.get("PATH", "") + os.pathsep + environment_cookie + str(i)
print("sys.executable: " + str(sys.executable))
exe = os.path.split(sys.executable)[1]
exe = exe.split(".")[0]
print("exe: " + str(exe))
polluted_process = subprocess.Popen([exe, "-c", "import time; time.sleep(3)"], env=os.environ.copy())
clean_process = subprocess.Popen([exe, "-c", "import time; time.sleep(3)"], env={})
print("polluted pid=" + str(polluted_process.pid))
print("clean pid=" + str(clean_process.pid))
pp = psutil.Process(polluted_process.pid)
pc = psutil.Process(clean_process.pid)
for i in range(0,8):
pp.environ()
for i in range(0,8):
e = pc.environ()
if len(e) > 0:
print("clean subprocess (should be empty) env=" + str(e))
exit(2)
print("no repro, try again :-/")
```
In order to better illustrate the problem, I added this debug output to `psutils`:
```diff
+++ psutil/arch/osx/process_info.c
@@ -263,6 +263,7 @@ psutil_get_environ(pid_t pid) {
goto error;
procargs = (char *)malloc(argmax);
+ printf("[psutil_get_environ] pid=%d, procargs=%p\n", pid, procargs);
if (NULL == procargs) {
PyErr_NoMemory();
goto error;
```
Example output:
```
$ python3 .py
sys.executable: /opt/homebrew/opt/python@3.10/bin/python3.10
exe: python3
polluted pid=63479
clean pid=63480
[psutil_get_environ] pid=63479, procargs=0x140088000
[psutil_get_environ] pid=63479, procargs=0x140188000
[psutil_get_environ] pid=63479, procargs=0x140088000
[psutil_get_environ] pid=63479, procargs=0x140188000
[psutil_get_environ] pid=63479, procargs=0x140088000
[psutil_get_environ] pid=63479, procargs=0x140188000
[psutil_get_environ] pid=63479, procargs=0x140088000
[psutil_get_environ] pid=63479, procargs=0x140188000
[psutil_get_environ] pid=63480, procargs=0x140088000
clean subprocess (should be empty) env={'EDITOR': 'vim', [[removed]], 'PATH': '/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:__MAGIC_ENV_COOKIE__0:__MAGIC_ENV_COOKIE__1:__MAGIC_ENV_COOKIE__2:__MAGIC_ENV_COOKIE__3:__MAGIC_ENV_COOKIE__4:__MAGIC_ENV_COOKIE__5:__MAGIC_ENV_COOKIE__6:__MAGIC_ENV_COOKIE__7:__MAGIC_ENV_COOKIE__8:__MAGIC_ENV_COOKIE__9:__MAGIC_ENV_COOKIE__10:__MAGIC_ENV_COOKIE__11:__MAGIC_ENV_COOKIE__12:__MAGIC_ENV_COOKIE__13:__MAGIC_ENV_COOKIE__14:__MAGIC_ENV_COOKIE__15:__MAGIC_ENV_COOKIE__16:__MAGIC_ENV_COOKIE__17:__MAGIC_ENV_COOKIE__18:__MAGIC_ENV_COOKIE__19:__MAGIC_ENV_COOKIE__20:__MAGIC_ENV_COOKIE__21:__MAGIC_ENV_COOKIE__22:__MAGIC_ENV_COOKIE__23:__MAGIC_ENV_COOKIE__24:__MAGIC_ENV_COOKIE__25:__MAGIC_ENV_COOKIE__26:__MAGIC_ENV_COOKIE__27:__MAGIC_ENV_COOKIE__28:__MAGIC_ENV_COOKIE__29:__MAGIC_ENV_COOKIE__30:__MAGIC_ENV_COOKIE__31:__MAGIC_ENV_COOKIE__32:__MAGIC_ENV_COOKIE__33:__MAGIC_ENV_COOKIE__34:__MAGIC_ENV_COOKIE__35:__MAGIC_ENV_COOKIE__36:__MAGIC_ENV_COOKIE__37:__MAGIC_ENV_COOKIE__38:__MAGIC_ENV_COOKIE__39:__MAGIC_ENV_COOKIE__40:__MAGIC_ENV_COOKIE__41:__MAGIC_ENV_COOKIE__42:__MAGIC_ENV_COOKIE__43:__MAGIC_ENV_COOKIE__44:__MAGIC_ENV_COOKIE__45:__MAGIC_ENV_COOKIE__46:__MAGIC_ENV_COOKIE__47:__MAGIC_ENV_COOKIE__48:__MAGIC_ENV_COOKIE__49:__MAGIC_ENV_COOKIE__50:__MAGIC_ENV_COOKIE__51:__MAGIC_ENV_COOKIE__52:__MAGIC_ENV_COOKIE__53:__MAGIC_ENV_COOKIE__54:__MAGIC_ENV_COOKIE__55:__MAGIC_ENV_COOKIE__56:__MAGIC_ENV_COOKIE__57:__MAGIC_ENV_COOKIE__58:__MAGIC_ENV_COOKIE__59:__MAGIC_ENV_COOKIE__60:__MAGIC_ENV_COOKIE__61:__MAGIC_ENV_COOKIE__62:__MAGIC_ENV_COOKIE__63:__MAGIC_ENV_COOKIE__64:__MAGIC_ENV_COOKIE__65:__MAGIC_ENV_COOKIE__66:__MAGIC_ENV_COOKIE__67:__MAGIC_ENV_COOKIE__68:__MAGIC_ENV_COOKIE__69:__MAGIC_ENV_COOKIE__70:__MAGIC_ENV_COOKIE__71:__MAGIC_ENV_COOKIE__72:__MAGIC_ENV_COOKIE__73:__MAGIC_ENV_COOKIE__74:__MAGIC_ENV_COOKIE__75:__MAGIC_ENV_COOKIE__76:__MAGIC_ENV_COOKIE__77:__MAGIC_ENV_COOKIE__78:__MAGIC_ENV_COOKIE__79:__MAGIC_ENV_COOKIE__80:__MAGIC_ENV_COOKIE__81:__MAGIC_ENV_COOKIE__82:__MAGIC_ENV_COOKIE__83:__MAGIC_ENV_COOKIE__84:__MAGIC_ENV_COOKIE__85:__MAGIC_ENV_COOKIE__86:__MAGIC_ENV_COOKIE__87:__MAGIC_ENV_COOKIE__88:__MAGIC_ENV_COOKIE__89:__MAGIC_ENV_COOKIE__90:__MAGIC_ENV_COOKIE__91:__MAGIC_ENV_COOKIE__92:__MAGIC_ENV_COOKIE__93:__MAGIC_ENV_COOKIE__94:__MAGIC_ENV_COOKIE__95:__MAGIC_ENV_COOKIE__96:__MAGIC_ENV_COOKIE__97:__MAGIC_ENV_COOKIE__98:__MAGIC_ENV_COOKIE__99:__MAGIC_ENV_COOKIE__100:__MAGIC_ENV_COOKIE__101:__MAGIC_ENV_COOKIE__102:__MAGIC_ENV_COOKIE__103:__MAGIC_ENV_COOKIE__104:__MAGIC_ENV_COOKIE__105:__MAGIC_ENV_COOKIE__106:__MAGIC_ENV_COOKIE__107:__MAGIC_ENV_COOKIE__108:__MAGIC_ENV_COOKIE__109:__MAGIC_ENV_COOKIE__110:__MAGIC_ENV_COOKIE__111:__MAGIC_ENV_COOKIE__112:__MAGIC_ENV_COOKIE__113:__MAGIC_ENV_COOKIE__114:__MAGIC_ENV_COOKIE__115:__MAGIC_ENV_COOKIE__116:__MAGIC_ENV_COOKIE__117:__MAGIC_ENV_COOKIE__118:__MAGIC_ENV_COOKIE__119:__MAGIC_ENV_COOKIE__120:__MAGIC_ENV_COOKIE__121:__MAGIC_ENV_COOKIE__122:__MAGIC_ENV_COOKIE__123:__MAGIC_ENV_COOKIE__124:__MAGIC_ENV_COOKIE__125:__MAGIC_ENV_COOKIE__126:__MAGIC_ENV_COOKIE__127:__MAGIC_ENV_COOKIE__128:__MAGIC_ENV_COOKIE__129:__MAGIC_ENV_COOKIE__130:__MAGIC_ENV_COOKIE__131:__MAGIC_ENV_COOKIE__132:__MAGIC_ENV_COOKIE__133:__MAGIC_ENV_COOKIE__134:__MAGIC_ENV_COOKIE__135:__MAGIC_ENV_COOKIE__136:__MAGIC_ENV_COOKIE__137:__MAGIC_ENV_COOKIE__138:__MAGIC_ENV_COOKIE__139:__MAGIC_ENV_COOKIE__140:__MAGIC_ENV_COOKIE__141:__MAGIC_ENV_COOKIE__142:__MAGIC_ENV_COOKIE__143:__MAGIC_ENV_COOKIE__144:__MAGIC_ENV_COOKIE__145:__MAGIC_ENV_COOKIE__146:__MAGIC_ENV_COOKIE__147:__MAGIC_ENV_COOKIE__148:__MAGIC_ENV_COOKIE__149:__MAGIC_ENV_COOKIE__150:__MAGIC_ENV_COOKIE__151:__MAGIC_ENV_COOKIE__152:__MAGIC_ENV_COOKIE__153:__MAGIC_ENV_COOKIE__154:__MAGIC_ENV_COOKIE__155:__MAGIC_ENV_COOKIE__156:__MAGIC_ENV_COOKIE__157:__MAGIC_ENV_COOKIE__158:__MAGIC_ENV_COOKIE__159:__MAGIC_ENV_COOKIE__160:__MAGIC_ENV_COOKIE__161:__MAGIC_ENV_COOKIE__162:__MAGIC_ENV_COOKIE__163:__MAGIC_ENV_COOKIE__164:__MAGIC_ENV_COOKIE__165:__MAGIC_ENV_COOKIE__166:__MAGIC_ENV_COOKIE__167:__MAGIC_ENV_COOKIE__168:__MAGIC_ENV_COOKIE__169:__MAGIC_ENV_COOKIE__170:__MAGIC_ENV_COOKIE__171:__MAGIC_ENV_COOKIE__172:__MAGIC_ENV_COOKIE__173:__MAGIC_ENV_COOKIE__174:__MAGIC_ENV_COOKIE__175:__MAGIC_ENV_COOKIE__176:__MAGIC_ENV_COOKIE__177:__MAGIC_ENV_COOKIE__178:__MAGIC_ENV_COOKIE__179:__MAGIC_ENV_COOKIE__180:__MAGIC_ENV_COOKIE__181:__MAGIC_ENV_COOKIE__182:__MAGIC_ENV_COOKIE__183:__MAGIC_ENV_COOKIE__184:__MAGIC_ENV_COOKIE__185:__MAGIC_ENV_COOKIE__186:__MAGIC_ENV_COOKIE__187:__MAGIC_ENV_COOKIE__188:__MAGIC_ENV_COOKIE__189:__MAGIC_ENV_COOKIE__190:__MAGIC_ENV_COOKIE__191:__MAGIC_ENV_COOKIE__192:__MAGIC_ENV_COOKIE__193:__MAGIC_ENV_COOKIE__194:__MAGIC_ENV_COOKIE__195:__MAGIC_ENV_COOKIE__196:__MAGIC_ENV_COOKIE__197:__MAGIC_ENV_COOKIE__198:__MAGIC_ENV_COOKIE__199'}
```
As you can see the allocated buffer for `pid=63480` matches with some instances of `pid=63479`.
Kudos to @naoufal450 and @gilles-duboscq for debugging this issue!
Signed-off-by: Bernhard Urban-Forster <bernhard.urban-forster@oracle.com>
Signed-off-by: Bernhard Urban-Forster <bernhard.urban-forster@oracle.com>
Co-authored-by: Giampaolo Rodola <g.rodola@gmail.com>
-rw-r--r-- | CREDITS | 5 | ||||
-rw-r--r-- | HISTORY.rst | 6 | ||||
-rw-r--r-- | psutil/arch/osx/process_info.c | 8 |
3 files changed, 13 insertions, 6 deletions
@@ -793,3 +793,8 @@ I: 2114 N: Chris Lalancette W: https://github.com/clalancette I: 2037 + +N: Bernhard Urban-Forster +C: Austria +W: https://github.com/lewurm +I: 2135 diff --git a/HISTORY.rst b/HISTORY.rst index f3c00fce..a0a3bb25 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -1,12 +1,14 @@ *Bug tracker at https://github.com/giampaolo/psutil/issues* -5.9.3 -===== +5.9.3 (IN DEVELOPMENT) +====================== XXXX-XX-XX **Bug fixes** +- 2135_, [macOS]: `Process.environ()`_ may contain garbage data. Fix + out-of-bounds read around ``sysctl_procargs``. (patch by Bernhard Urban-Forster) - 2142_, [POSIX]: `net_if_stats()`_ 's ``flags`` on Python 2 returned unicode instead of str. (patch by Matthieu Darbois) diff --git a/psutil/arch/osx/process_info.c b/psutil/arch/osx/process_info.c index 3a446a62..4af510e8 100644 --- a/psutil/arch/osx/process_info.c +++ b/psutil/arch/osx/process_info.c @@ -116,14 +116,14 @@ psutil_sysctl_argmax() { // Read process argument space. static int -psutil_sysctl_procargs(pid_t pid, char *procargs, size_t argmax) { +psutil_sysctl_procargs(pid_t pid, char *procargs, size_t *argmax) { int mib[3]; mib[0] = CTL_KERN; mib[1] = KERN_PROCARGS2; mib[2] = pid; - if (sysctl(mib, 3, procargs, &argmax, NULL, 0) < 0) { + if (sysctl(mib, 3, procargs, argmax, NULL, 0) < 0) { if (psutil_pid_exists(pid) == 0) { NoSuchProcess("psutil_pid_exists -> 0"); return 1; @@ -188,7 +188,7 @@ psutil_get_cmdline(pid_t pid) { goto error; } - if (psutil_sysctl_procargs(pid, procargs, argmax) != 0) + if (psutil_sysctl_procargs(pid, procargs, &argmax) != 0) goto error; arg_end = &procargs[argmax]; @@ -268,7 +268,7 @@ psutil_get_environ(pid_t pid) { goto error; } - if (psutil_sysctl_procargs(pid, procargs, argmax) != 0) + if (psutil_sysctl_procargs(pid, procargs, &argmax) != 0) goto error; arg_end = &procargs[argmax]; |