summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBernhard Urban-Forster <lewurm@gmail.com>2022-09-19 12:55:00 +0200
committerGitHub <noreply@github.com>2022-09-19 12:55:00 +0200
commit5d46b358d31ff7f0a67b104ad4cffffa032b3b35 (patch)
tree39b4b1307225933e068742a493a1a84420ad9cbb
parent9c38dd74e8a393371e6547b9404c303fa85a1459 (diff)
downloadpsutil-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--CREDITS5
-rw-r--r--HISTORY.rst6
-rw-r--r--psutil/arch/osx/process_info.c8
3 files changed, 13 insertions, 6 deletions
diff --git a/CREDITS b/CREDITS
index 7d1e7a69..93866be5 100644
--- a/CREDITS
+++ b/CREDITS
@@ -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];