Fix: copy sockinfo into local buffer before in-place split in mt_connect - #2
Open
edmundlod wants to merge 1 commit into
Open
Fix: copy sockinfo into local buffer before in-place split in mt_connect#2edmundlod wants to merge 1 commit into
edmundlod wants to merge 1 commit into
Conversation
mt_connect() stored lua_tostring()'s return in a const char *sockinfo and,
after lua_pop() had popped the argument, dispatched on the connection
string by NUL-terminating its protocol prefix ('inet'/'unix'/'local') and
host part in place (*p = '\0' / *at = '\0'). lua_tostring() returns a
pointer into Lua-owned storage that the pop may reclaim or share with
other interned strings, so both mutating it and reading it afterwards
(e.g. the strlcpy into sa.sun_path and the connect() error string) are
undefined behaviour.
Replace the const char *sockinfo pointer with a char sockinfo[BUFRSZ]
stack buffer populated via strlcpy() before lua_pop(). The in-place ':'
and '@' splits now operate on memory we own. No call-site changes.
Ported from the PhoenixDKIM fork of OpenDKIM.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In
mt_connect(),sockinfoholds the pointer returned bylua_tostring(l, 1). Afterlua_pop(l, top)runs, the connection string is dispatched by NUL-terminating its protocol prefix (inet/unix/local) and host part in place (*p = '\0',*at = '\0').lua_tostring()returns a pointer into Lua-owned storage that the pop may reclaim or share with other interned strings. So both mutating that storage and reading it afterwards (thestrlcpyintosa.sun_path, and theconnect()error string) are undefined behaviour.This replaces the
const char *sockinfopointer with achar sockinfo[BUFRSZ]stack buffer populated viastrlcpy()beforelua_pop(). The in-place':'and'@'splits then operate on memory we own. No call-site changes; builds clean viaautoreconf -fvi && ./configure && make.Found and fixed in the PhoenixDKIM fork of OpenDKIM; sending it back upstream.