FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > ArchLinux > ArchLinux Development

 
 
LinkBack Thread Tools
 
Old 11-05-2009, 03:53 PM
Firmicus
 
Default makechrootpkg: Use the host's SRCDEST and PKGDEST if they are defined

Aaron Griffin wrote:
> On Wed, Nov 4, 2009 at 8:02 PM, Eric Bélanger <snowmaniscool@gmail.com> wrote:
>
>> Signed-off-by: Eric Bélanger <snowmaniscool@gmail.com>
>> ---
>> makechrootpkg | 3 +++
>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/makechrootpkg b/makechrootpkg
>> index 5095425..d1dcf32 100755
>> --- a/makechrootpkg
>> +++ b/makechrootpkg
>> @@ -150,6 +150,9 @@ if [ "$REPACK" != "1" ]; then
>> rm -rf "$uniondir/build/"*
>> fi
>>
>> +eval $(grep '^SRCDEST=' /etc/makepkg.conf)
>> +eval $(grep '^PKGDEST=' /etc/makepkg.conf)
>> +
>> [ -d "$uniondir/pkgdest" ] || mkdir "$uniondir/pkgdest"
>> if ! grep "PKGDEST=/pkgdest" "$uniondir/etc/makepkg.conf" >/dev/null 2>&1; then
>> echo "Setting PKGDEST in makepkg.conf"
>>
>
> The eval seems slightly dangerous to me... does anyone else have this
> concern, or am I being too careful?
>
>

I agree. It is more than "slightly" dangerous. If makepkg.conf contained
a line such as:
PKGDEST="blabla" && rm -rf /
then the eval would indeed execute "rm -rf /". This is particularly bad
since makechrootpkg is called with sudo ...

Better look for a safer alternative. What about this?

PKGDEST=$(source /etc/makepkg.conf && echo $PKGDEST)
SRCDEST=$(source /etc/makepkg.conf && echo $SRCDEST)

This is not very pretty as we need to source makepkg.conf twice, but at
least it is safer.
We could also do something like:
ORIGDESTDIRS=$(source /etc/makepkg.conf && echo $PKGDEST $SRCDEST)
PKGDEST=$(echo $ORIGDESTDIRS | cut -d' ' -f1)
SRCDEST=$(echo $ORIGDESTDIRS | cut -d' ' -f2)
 
Old 11-05-2009, 03:57 PM
Firmicus
 
Default makechrootpkg: Use the host's SRCDEST and PKGDEST if they are defined

Daenyth Blank wrote:
> On Thu, Nov 5, 2009 at 12:05, Aaron Griffin <aaronmgriffin@gmail.com> wrote:
>
>> The eval seems slightly dangerous to me... does anyone else have this
>> concern, or am I being too careful?
>>
>>
>
> eval is always dangerous. In this case, however, it's eval-ing from a
> text file only writable by root. If an attacker has root write
> permissions, you have more to worry about than this.
>
>
True, but I still prefer to be extra careful, as /etc/makepkg.conf might
have been compromised through other channels.
 
Old 11-05-2009, 04:03 PM
Firmicus
 
Default makechrootpkg: Use the host's SRCDEST and PKGDEST if they are defined

Daenyth Blank wrote:
> On Thu, Nov 5, 2009 at 12:53, Firmicus <Firmicus@gmx.net> wrote:
>
>> Better look for a safer alternative. What about this?
>>
>> PKGDEST=$(source /etc/makepkg.conf && echo $PKGDEST)
>>
> That's actually more dangerous than the eval, since you are
> essentially eval-ing the entire file.
>
>
Again you're right. Somehow I thought source only affected variables,
but actually it does also eval each line of the sourced file. Sorry for
the noise.
 
Old 11-05-2009, 06:15 PM
RogutÄ—s Sparnuotos
 
Default makechrootpkg: Use the host's SRCDEST and PKGDEST if they are defined

Daenyth Blank (2009-11-05 13:07):
> On Thu, Nov 5, 2009 at 13:03, Aaron Griffin <aaronmgriffin@gmail.com> wrote:
> > I was thinking more along the lines of:
> >
> > Original: eval $(grep '^SRCDEST=' /etc/makepkg.conf)
> >
> > SRCDEST=$(grep '^SRCDEST=' /etc/makepkg.conf | cut -d= -f2)
> > PKGDEST=$(grep '^PKGDEST=' /etc/makepkg.conf | cut -d= -f2)
> >
>
> I believe that should work... Make sure to throw in a -d -w check
> after, to make sure it's right... I think a malicious line (".. && rm
> -rf /") would simply get stored as a string.. as long as we quote
> everything properly and unset it if it's not right, I'm pretty sure
> it's not dangerous. It could however, fail in cases where it's split
> over multiple lines. I don't think that's a reason not to do it
> though, I can't imagine a reason for such a thing.

This wouldn't work for
SRCDEST="/path/to/dir"
SRCDEST='/path/to/dir'

But sed should be able to do it:
SRCDEST=$(sed -nr "/^SRCDEST=/{s/[^=]+=(['"]?)(.+)1/2/p}" /etc/makepkg.conf)

--
-- RogutÄ—s Sparnuotos
 
Old 11-20-2009, 06:50 AM
Firmicus
 
Default makechrootpkg: Use the host's SRCDEST and PKGDEST if they are defined

Aaron Griffin wrote:
>> Aaron Griffin wrote:
>>
>>> On Wed, Nov 4, 2009 at 8:02 PM, Eric Bélanger <snowmaniscool@gmail.com> wrote:
>>>
>>>
>>>> Signed-off-by: Eric Bélanger <snowmaniscool@gmail.com>
>>>> ---
>>>> makechrootpkg | 3 +++
>>>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/makechrootpkg b/makechrootpkg
>>>> index 5095425..d1dcf32 100755
>>>> --- a/makechrootpkg
>>>> +++ b/makechrootpkg
>>>> @@ -150,6 +150,9 @@ if [ "$REPACK" != "1" ]; then
>>>> rm -rf "$uniondir/build/"*
>>>> fi
>>>>
>>>> +eval $(grep '^SRCDEST=' /etc/makepkg.conf)
>>>> +eval $(grep '^PKGDEST=' /etc/makepkg.conf)
>>>> +
>>>> [ -d "$uniondir/pkgdest" ] || mkdir "$uniondir/pkgdest"
>>>> if ! grep "PKGDEST=/pkgdest" "$uniondir/etc/makepkg.conf" >/dev/null 2>&1; then
>>>> echo "Setting PKGDEST in makepkg.conf"
>>>>
>>>>
> <snip>
>
> I was thinking more along the lines of:
>
> Original: eval $(grep '^SRCDEST=' /etc/makepkg.conf)
>
> SRCDEST=$(grep '^SRCDEST=' /etc/makepkg.conf | cut -d= -f2)
> PKGDEST=$(grep '^PKGDEST=' /etc/makepkg.conf | cut -d= -f2)
>
>

Re: Eric's request of November 11 in another thread, I am in favour of
committing the above fix to devtools, and then release a new version. Is
it ok with you Aaron?

F
 

Thread Tools




All times are GMT. The time now is 10:48 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright ©2007 - 2008, www.linux-archive.org