Am 28.07.25 um 11:59 AM schrieb Fabian Grünbichler:
> On July 25, 2025 5:48 pm, Friedrich Weber wrote:
>> Without untainting, offline-deleting a volume-chain snapshot on a
>> directory storage via the GUI fails with an "Insecure dependecy in
>> exec [...]" error, because volume_snapshot_delete uses the filename
>> its qemu-img invocation.
>>
>> Signed-off-by: Friedrich Weber <f.we...@proxmox.com>
>> ---
>>
>> Notes:
>> I'm not too familiar with the taint mode. Allowing anything that
>> starts with a slash seems a little lax, but I don't know if we can do
>> any meaningful validation here -- let me know if we can.
>>
>> src/PVE/Storage/Plugin.pm | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
>> index a817186..2bd05bd 100644
>> --- a/src/PVE/Storage/Plugin.pm
>> +++ b/src/PVE/Storage/Plugin.pm
>> @@ -1789,6 +1789,7 @@ sub volume_snapshot_info {
>> my $snapshots = $json_decode;
>> for my $snap (@$snapshots) {
>> my $snapfile = $snap->{filename};
>> + ($snapfile) = $snapfile =~ m|^(/.*)|; # untaint
>
> we also validate that the path matches our naming scheme below, but that
> is mostly concerned with the final component..
>
> I called out that the references for backing images are not relative in
> a previous iteration of the qcow2 patch series, it seems that slipped
> through?
>
> right now, it's not possible to change the backing directory path of the
> storage, or the LVM VG without breaking all snapshot chains stored
> there because all the back references to snapshots are using absolute
> paths instead of relative ones..
>
> if we fix that (and we probably should?), then the untainting RE here
> would become wrong again..
Agreed, making the backing paths relative (for new volumes) sounds
sensible and then we can also validate them better :)
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
