Poor use of /tmp needs to verify the integrity of downloaded archive

[codyro]

Added basic user check for downloaded archive. Should have better checking / hash checking ultimately. Bug report submitted via Help Desk.

[cloudflare]

Thanks a lot for the bug report. I've pushed a new version of the
cpanel package to correct (23912f3).
Let me know what you think.

Cheers,

Ian

On Mon, Oct 31, 2011 at 9:40 AM, Cody Robertson
reply@reply.github.com
wrote:

Added basic user check for downloaded archive. Should have better checking / hash checking ultimately. Bug report submitted via Help Desk.

You can merge this Pull Request by running:

 git pull https://github.com/codyro/CloudFlare-CPanel master

Or you can view, comment on it, or merge it online at:

 #3

-- Commit Summary --

• Added basic user check for downloaded archive. Should have better checking / hash checking

-- File Changes --

M cloudflare/bin/cloudflare_update.sh (6)

-- Patch Links --

 https://github.com/cloudflare/CloudFlare-CPanel/pull/3.patch
 https://github.com/cloudflare/CloudFlare-CPanel/pull/3.diff

Reply to this email directly or view it on GitHub:
#3

[codyro]

That should work fine although there is some goofy stuff in there. I don't see the point of downloading the tarball to a temp directory then moving it to a non-random folder (/usr/local/cpanel/cloudflare_tmp). It should stay in the temp directory throughout the whole process? I can only imagine this was done to bypass noexec?

[cloudflare]

On Tue, Nov 1, 2011 at 9:33 AM, Cody Robertson
reply@reply.github.com
wrote:

That should work fine although there is some goofy stuff in there. I don't see the point of downloading the tarball to a temp directory then moving it to a non-random folder (/usr/local/cpanel/cloudflare_tmp). It should stay in the temp directory throughout the whole process? I can only imagine this was done to bypass noexec?

Exactly -- bypassing noexec on /tmp.

Reply to this email directly or view it on GitHub:
#3 (comment)

[codyro]

That makes sense. I'm not familiar with perl although I suspect with the tempdir() function you could probably just create a temporary directory in /usr/local/cpanel from the start instead of relying on any static folders / shuffling stuff around. Reduces the complexity slightly without changing the logic at all.

Closing this issue however as it appears to be fixed. Just nit picking at this point :).

[cloudflare]

On Tue, Nov 1, 2011 at 1:01 PM, Cody Robertson
reply@reply.github.com
wrote:

That makes sense. I'm not familiar with perl although I suspect with the tempdir() function you could probably just create a temporary directory in /usr/local/cpanel from the start instead of relying on any static folders / shuffling stuff around. Reduces the complexity slightly without changing the logic at all.

Closing this issue however as it appears to be fixed. Just nit picking at this point :).

Agreed, the code base could stand to be cleaned up a bit ;). This is
what users are for.

Reply to this email directly or view it on GitHub:
#3 (comment)