Skip to content

Make sure that make doesn't rm -rf the system out of existence#47121

Merged
thaJeztah merged 1 commit intomoby:masterfrom
voloder:master
Jan 23, 2024
Merged

Make sure that make doesn't rm -rf the system out of existence#47121
thaJeztah merged 1 commit intomoby:masterfrom
voloder:master

Conversation

@voloder
Copy link
Contributor

@voloder voloder commented Jan 19, 2024

- What I did

Ensure that make raises an error if mktemp, for whatever reason, returns an empty string. This could possibly happen if, for some reason, the /tmp directory was chown'd or there was no free space left on the system.

This pull request makes sure that it won't rm -rf /* the system out of existence in case $@_TMP_OUT is an empty string.

If $($@_TMP_OUT) was empty,

rm -rf "$($@_TMP_OUT)"/*

would evaluate to

rm -rf /*

What would happen next is up to your imagination

- How I did it

I did it by adding

ifeq ($($@_TMP_OUT),)
    $(error Could not create temp directory.)
endif

after

$(eval $@_TMP_OUT := $(shell mktemp -d -t moby-output.XXXXXXXXXX))

in the Makefile

- How to verify it

Run make generate-files

- Description for the changelog

  • Fix potential system wipe

Signed-off-by: voloder <110066198+voloder@users.noreply.github.com>
@voloder
Copy link
Contributor Author

voloder commented Jan 22, 2024

I tried to find conditions in which this would occur "normally". I only got so far as to chmod'ing the tmp directory and getting $($@_TMP_OUT) to be an empty string. The buildx command would fail because then the dist option in the command would be empty. Nonetheless i think it's good to at the very least include a check and return an appropriate error message if the temp directory failed to have been created. I am not sure if there are any conditions that would make the buildx command return 0 despite dist being empty.

@thaJeztah
Copy link
Member

Thanks! Let's bring this one in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants