-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[macOS] Fix out-of-bounds read around sysctl_procargs #2135
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[macOS] Fix out-of-bounds read around sysctl_procargs #2135
Conversation
517baba
to
33c10f1
Compare
@@ -1,8 +1,14 @@ | |||
*Bug tracker at https://github.com/giampaolo/psutil/issues* | |||
|
|||
5.9.3 (IN DEVELOPMENT) | |||
====================== |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@giampaolo I saw you just released 5.9.2
. Is that entry okay like this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup, that's fine
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/[email protected]/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 <[email protected]>
33c10f1
to
ee179d8
Compare
Hey @giampaolo, does this fix look good to you? |
Thank you 🙂 |
Thank you for the fix. It wasn't easy to spot and debug. (I know I wouldn't have been able to hehe) |
Summary
Description
The buffer size is initially determined by querying
argmax
:psutil/psutil/arch/osx/process_info.c
Lines 260 to 265 in 9f9a82d
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:
In order to better illustrate the problem, I added this debug output to
psutils
:Example output:
As you can see the allocated buffer for
pid=63480
matches with some instances ofpid=63479
.Kudos to @naoufal450 and @gilles-duboscq for debugging this issue!