From d05c84cf3a255c110ce98195a3dee2cb3af1a71a Mon Sep 17 00:00:00 2001 From: Marc Espie Date: Sat, 2 Apr 2016 12:18:45 +0000 Subject: streamline the code that checks meta information before extracting files - verify_modes should only verify modes - put the whole checking code in one single routine in validate_meta, makes the size checking code more obvious - document what's going on - prevent PkgCreate from creating impossible packages, only a few Ustar objects are actually usable. --- usr.sbin/pkg_add/OpenBSD/Add.pm | 24 +--------- usr.sbin/pkg_add/OpenBSD/ArcCheck.pm | 89 +++++++++++++++++++++++++---------- usr.sbin/pkg_add/OpenBSD/PkgCreate.pm | 5 +- 3 files changed, 71 insertions(+), 47 deletions(-) diff --git a/usr.sbin/pkg_add/OpenBSD/Add.pm b/usr.sbin/pkg_add/OpenBSD/Add.pm index fc523442273..96f38dce79e 100644 --- a/usr.sbin/pkg_add/OpenBSD/Add.pm +++ b/usr.sbin/pkg_add/OpenBSD/Add.pm @@ -1,5 +1,5 @@ # ex:ts=8 sw=4: -# $OpenBSD: Add.pm,v 1.169 2015/05/25 07:20:31 espie Exp $ +# $OpenBSD: Add.pm,v 1.170 2016/04/02 12:18:44 espie Exp $ # # Copyright (c) 2003-2014 Marc Espie # @@ -456,27 +456,7 @@ sub prepare_to_extract my $destdir = $state->{destdir}; $file->{cwd} = $self->cwd; - if (defined $self->{symlink} || $file->isSymLink) { - unless (defined $self->{symlink} && $file->isSymLink) { - $state->fatal("bogus symlink #1", $self->name); - } - if (!$file->check_linkname($self->{symlink})) { - $state->fatal("archive symlink does not match #1 != #2", - $file->{linkname}, $self->{symlink}); - } - } elsif (defined $self->{link} || $file->isHardLink) { - unless (defined $self->{link} && $file->isHardLink) { - $state->fatal("bogus hardlink #1", $self->name); - } - if (!$file->check_linkname($self->{link})) { - $state->fatal("archive hardlink does not match #1 != #2", - $file->{linkname}, $self->{link}); - } - } elsif (!$file->isFile) { - $state->fatal("archive content for #1 should be file", - $self->name); - } - if (!$file->verify_modes($self)) { + if (!$file->validate_meta($self)) { $state->fatal("can't continue"); } diff --git a/usr.sbin/pkg_add/OpenBSD/ArcCheck.pm b/usr.sbin/pkg_add/OpenBSD/ArcCheck.pm index 0c62aa8d9b0..0f5fad9a4f4 100644 --- a/usr.sbin/pkg_add/OpenBSD/ArcCheck.pm +++ b/usr.sbin/pkg_add/OpenBSD/ArcCheck.pm @@ -1,5 +1,5 @@ # ex:ts=8 sw=4: -# $OpenBSD: ArcCheck.pm,v 1.31 2014/12/23 08:46:31 espie Exp $ +# $OpenBSD: ArcCheck.pm,v 1.32 2016/04/02 12:18:44 espie Exp $ # # Copyright (c) 2005-2006 Marc Espie # @@ -16,10 +16,16 @@ # OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. # Supplementary code to handle archives in the package context. -# Contrarily to GNU-tar, we do not change the archive format, but by -# convention, the names LongName\d+ and LongLink\d correspond to names -# too long to fit. The actual names reside in the PLIST, but the archive -# is still a valid archive. +# Ustar allows about anything, but we want to forbid a lot of things. +# this code is used during creation and extraction +# specifically, during create time: +# - prevent a lot of weird objects from entering the archives +# - make sure all relevant users/modes are recorded in the PLIST item + +# during extraction: +# - make sure complex objects have all their relevant properties recorded +# - disallow extraction of non-files/links. +# - guard against files much longer than they should be. use strict; use warnings; @@ -27,15 +33,11 @@ use warnings; use OpenBSD::Ustar; package OpenBSD::Ustar::Object; +use POSIX; -# match archive header name against PackingElement item -sub check_name -{ - my ($self, $item) = @_; - return $self->name eq $item->name; -} +sub is_allowed() { 0 } -# match archive header link name against actual link names +# match archive header link name against actual link name sub check_linkname { my ($self, $linkname) = @_; @@ -46,7 +48,48 @@ sub check_linkname return $c eq $linkname; } -use POSIX; +sub validate_meta +{ + my ($o, $item) = @_; + + $o->{cwd} = $item->cwd; + if (defined $item->{symlink} || $o->isSymLink) { + unless (defined $item->{symlink} && $o->isSymLink) { + $o->errsay("bogus symlink #1", $item->name); + return 0; + } + if (!$o->check_linkname($item->{symlink})) { + $o->errsay("archive symlink does not match #1 != #2", + $o->{linkname}, $item->{symlink}); + return 0; + } + } elsif (defined $item->{link} || $o->isHardLink) { + unless (defined $item->{link} && $o->isHardLink) { + $o->errsay("bogus hardlink #1", $item->name); + return 0; + } + if (!$o->check_linkname($item->{link})) { + $o->errsay("archive hardlink does not match #1 != #2", + $o->{linkname}, $item->{link}); + return 0; + } + } elsif ($o->isFile) { + if (!defined $item->{size}) { + $o->errsay("Error: file #1 does not have recorded size", + $item->fullname); + return 0; + } elsif ($item->{size} != $o->{size}) { + $o->errsay("Error: size does not match for #1", + $item->fullname); + return 0; + } + } else { + $o->errsay("archive content for #1 should be file", + $item->name); + return 0; + } + return $o->verify_modes($item); +} sub verify_modes { @@ -82,20 +125,18 @@ sub verify_modes $result = 0; } } - if ($o->isFile) { - if (!defined $item->{size}) { - $o->errsay("Error: file #1 does not have recorded size", - $item->fullname); - $result = 0; - } elsif ($item->{size} != $o->{size}) { - $o->errsay("Error: size does not match for #1", - $item->fullname); - $result = 0; - } - } return $result; } +package OpenBSD::Ustar::HardLink; +sub is_allowed() { 1 } + +package OpenBSD::Ustar::SoftLink; +sub is_allowed() { 1 } + +package OpenBSD::Ustar::File; +sub is_allowed() { 1 } + package OpenBSD::Ustar; use POSIX; diff --git a/usr.sbin/pkg_add/OpenBSD/PkgCreate.pm b/usr.sbin/pkg_add/OpenBSD/PkgCreate.pm index 3487638c716..45d3a42d3a9 100644 --- a/usr.sbin/pkg_add/OpenBSD/PkgCreate.pm +++ b/usr.sbin/pkg_add/OpenBSD/PkgCreate.pm @@ -1,6 +1,6 @@ #! /usr/bin/perl # ex:ts=8 sw=4: -# $OpenBSD: PkgCreate.pm,v 1.118 2015/11/06 08:53:12 espie Exp $ +# $OpenBSD: PkgCreate.pm,v 1.119 2016/04/02 12:18:44 espie Exp $ # # Copyright (c) 2003-2014 Marc Espie # @@ -294,6 +294,9 @@ sub prepare_for_archival if (!$o->verify_modes($self)) { $state->error("modes don't match for #1", $self->fullname); } + if (!$o->is_allowed) { + $state->error("can't package #1", $self->fullname); + } return $o; } -- cgit v1.2.3