[PATCH openbmc v6 17/18] initfs: update: Sanitze whitelist directory entries

[Andrew Jeffery]

On Wed, 2016-06-22 at 19:30 -0500, OpenBMC Patches wrote:
> From: Milton Miller <miltonm at us.ibm.com>
> 
> Repeatedly strip trailing / and /. from whitelist entries and
> fail if an entry includes /../ or ends with /.. or doesn't start
> with a /.  Also use the entries quoted to avoid any glob.

Right. Ending this particular sentence with a '/' is a bit subtle. I
think you should quote your path bits: "/", "/.", "/../", "/.."; this
way the sentence would end '... or doesn't start with a "/".'.
> 
> It was noticed the save code was saving directories that ended in /
> into a subdirectory of the last component name.  This was traced
> the the code creating the directory just stripping the last /
> and then copying to the directory.
> 
> Choose to sanitize the entry where possible for ease of use verses
> a small performance penality.
> 
> Signed-off-by: Milton Miller <miltonm at us.ibm.com>
> ---
>  .../obmc-phosphor-initfs/files/obmc-update.sh             | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-update.sh b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-update.sh
> index 7120a18..bfacd00 100755
> --- a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-update.sh
> +++ b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-update.sh
> @@ -170,13 +170,24 @@ then
>  
>  	while read f
>  	do
> -		if ! test -e $upper/$f
> +		# Entries shall start with /, no trailing /.. or embedded /../
> +		if test "/${f#/}" != "$f" -o "${f%/..}" != "${f#*/../}"

Can we do this with `readlink -f` or `realpath` instead? Do we have
them available in this environment? As in, test that the result before
and after realpath are the same. You can tell realpath to not expand
symlinks.

> +		then
> +			echo 1>&2 "WARNING: Skipping bad whitelist entry $f."
> +			continue
> +		fi
> +		if ! test -e "$upper/$f"
>  		then
>  			continue
>  		fi
>  		d="$save/$f"
> +		while test "${d%/}" != "${d%/.}"
> +		do
> +			d="${d%/.}"
> +			d="${d%/}"
> +		done
>  		mkdir -p "${d%/*}"
> -		cp -rp $upper/$f "${d%/*}/"
> +		cp -rp "$upper/$f" "${d%/*}/"

Again the while loop and other bits can hopefully be replaced by
`readlink -f` or `realpath`. The whole trailing-slash thing with `cp`
always throws me.

Andrew

>  	done < $whitelist
>  
>  	if test -n "$mounted"
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20160623/bac48adb/attachment.sig>