Skip to content

Commit

Permalink
Avoid potential buffer overflow/missing zero termination of string (#…
Browse files Browse the repository at this point in the history
…36408)

Co-authored-by: Elliot Saba <staticfloat@gmail.com>
  • Loading branch information
martinholters and staticfloat authored Jun 28, 2020
1 parent fe934f0 commit 9d71d37
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/dlload.c
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,8 @@ JL_DLLEXPORT void *jl_load_dynamic_library(const char *modname, unsigned flags,
snprintf(relocated, PATHBUF, "%s%s", jl_options.julia_bindir, dl_path + 16);
len = len - 16 + strlen(jl_options.julia_bindir);
} else {
strncpy(relocated, dl_path, len);
strncpy(relocated, dl_path, PATHBUF);
relocated[PATHBUF-1] = '\0';
}
for (i = 0; i < n_extensions; i++) {
const char *ext = extensions[i];
Expand Down
50 changes: 50 additions & 0 deletions stdlib/Libdl/test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -222,4 +222,54 @@ let dl = C_NULL
@test_skip !Libdl.dlclose(dl) # Syscall doesn't fail on Win32
end

# test DL_LOAD_PATH handling and @executable_path expansion
mktempdir() do dir
# Create a `libdcalltest` in a directory that is not on our load path
src_path = joinpath(private_libdir, "libccalltest.$(Libdl.dlext)")
dst_path = joinpath(dir, "libdcalltest.$(Libdl.dlext)")
cp(src_path, dst_path)

# Add an absurdly long entry to the load path to verify it doesn't lead to a buffer overflow
push!(Base.DL_LOAD_PATH, joinpath(dir, join(rand('a':'z', 10000))))

# Add the temporary directors to load path by absolute path
push!(Base.DL_LOAD_PATH, dir)

# Test that we can now open that file
Libdl.dlopen("libdcalltest") do dl
fptr = Libdl.dlsym(dl, :set_verbose)
@test fptr !== nothing
@test_throws ErrorException Libdl.dlsym(dl, :foo)

fptr = Libdl.dlsym_e(dl, :set_verbose)
@test fptr != C_NULL
fptr = Libdl.dlsym_e(dl, :foo)
@test fptr == C_NULL
end

# Skip these tests if the temporary directory is not on the same filesystem
# as the BINDIR, as in that case, a relative path will never work.
if Base.Filesystem.splitdrive(dir)[1] != Base.Filesystem.splitdrive(Sys.BINDIR)[1]
return
end

empty!(Base.DL_LOAD_PATH)
push!(Base.DL_LOAD_PATH, joinpath(dir, join(rand('a':'z', 10000))))

# Add this temporary directory to our load path, now using `@executable_path` to do so.
push!(Base.DL_LOAD_PATH, joinpath("@executable_path", relpath(dir, Sys.BINDIR)))

# Test that we can now open that file
Libdl.dlopen("libdcalltest") do dl
fptr = Libdl.dlsym(dl, :set_verbose)
@test fptr !== nothing
@test_throws ErrorException Libdl.dlsym(dl, :foo)

fptr = Libdl.dlsym_e(dl, :set_verbose)
@test fptr != C_NULL
fptr = Libdl.dlsym_e(dl, :foo)
@test fptr == C_NULL
end
end

end

3 comments on commit 9d71d37

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(ALL, isdaily = true)

Please sign in to comment.