Skip to content
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

Implement file path sanitation #273

Merged
merged 1 commit into from
Nov 23, 2023

Conversation

henrybear327
Copy link
Collaborator

@henrybear327 henrybear327 commented Nov 22, 2023

As mentioned in issue #137, the elf path is not sanitized before opening, which might be a security risk. This PR addresses this issue by porting the path sanitation logic from Golang to C.

Reference code: https://cs.opensource.google/go/go/+/refs/tags/go1.21.4:src/path/path.go;l=70

Unit test -> #274

src/utils.c Fixed Show fixed Hide fixed
src/utils.c Fixed Show resolved Hide resolved
src/utils.c Fixed Show resolved Hide resolved
src/utils.c Fixed Show fixed Hide fixed
src/utils.c Fixed Show fixed Hide fixed
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

Cppcheck (reported by Codacy) found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

@henrybear327 henrybear327 force-pushed the feat/elf_path_check branch 2 times, most recently from c501e2e to 28f31a1 Compare November 22, 2023 23:11
src/utils.c Outdated Show resolved Hide resolved
@jserv jserv changed the title Implement file path sanitation (Issue #137) Implement file path sanitation Nov 23, 2023
Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Merge the proposed changes in #274

@henrybear327 henrybear327 force-pushed the feat/elf_path_check branch 2 times, most recently from 5b30fbd to da86b50 Compare November 23, 2023 06:35
@henrybear327
Copy link
Collaborator Author

henrybear327 commented Nov 23, 2023

Merge the proposed changes in #274

Merged using a separate commit.

jserv

This comment was marked as resolved.

src/elf.c Outdated Show resolved Hide resolved
src/main.c Outdated Show resolved Hide resolved
src/utils.c Outdated Show resolved Hide resolved
src/utils.c Outdated Show resolved Hide resolved
jserv

This comment was marked as resolved.

Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Check mk/tests.mk for existing unit test programs along with mock functions.

@henrybear327 henrybear327 force-pushed the feat/elf_path_check branch 2 times, most recently from d108622 to 52e1dab Compare November 23, 2023 07:29
src/elf.c Outdated Show resolved Hide resolved
tests/path/test-path.c Outdated Show resolved Hide resolved
src/utils.c Outdated Show resolved Hide resolved
tests/path/test-path.c Outdated Show resolved Hide resolved
@henrybear327 henrybear327 force-pushed the feat/elf_path_check branch 2 times, most recently from 34daec4 to 6d3bc33 Compare November 23, 2023 07:50
src/utils.c Outdated Show resolved Hide resolved
@henrybear327 henrybear327 force-pushed the feat/elf_path_check branch 2 times, most recently from dbd1d4a to 683a5bc Compare November 23, 2023 07:54
jserv

This comment was marked as outdated.

src/utils.c Show resolved Hide resolved
src/utils.c Show resolved Hide resolved
src/utils.c Outdated Show resolved Hide resolved
src/utils.h Outdated Show resolved Hide resolved
@henrybear327 henrybear327 force-pushed the feat/elf_path_check branch 3 times, most recently from f889251 to 5c5286b Compare November 23, 2023 08:12
@jserv
Copy link
Contributor

jserv commented Nov 23, 2023

Is it time to close #137? If so, append Close #137 at the end of git commit message.

@henrybear327 henrybear327 force-pushed the feat/elf_path_check branch 2 times, most recently from 1df5044 to c7d1f74 Compare November 23, 2023 08:18
@henrybear327
Copy link
Collaborator Author

Is it time to close #137? If so, append Close #137 at the end of git commit message.

Yes. Commit messages changed as requested

src/utils.c Outdated Show resolved Hide resolved
src/utils.c Outdated Show resolved Hide resolved
@jserv jserv merged commit 2c096fb into sysprog21:master Nov 23, 2023
16 checks passed
@henrybear327 henrybear327 deleted the feat/elf_path_check branch November 23, 2023 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants