Bug#880368: YAML::XS::Load expects utf8 octets, not perl's encoding; use slurp_raw

[Dominique Dumont]

On Thursday, 19 December 2019 10:18:12 CET Dominique Dumont wrote:
> provided
> $YAML::XS::LoadBlessed is set to 0, which is not a problem for cme

Surprise.. 

Thanks to this test [1], it turns out that setting LoadBlessed with 
a local does not work (i.e. "local $YAML::XS::LoadBlessed = 0;").

Only "$YAML::XS::LoadBlessed = 0" without local disables the feature that
loads blessed objects. 

Which means I need to rework Config::Model::Backend::YAML :-/

All the best

[1] 
https://salsa.debian.org/perl-team/modules/packages/libconfig-model-dpkg-perl/commit/481b3df20f23d24e8021c4f100d66fe4c52fbabb

Bug#880368: YAML::XS::Load expects utf8 octets, not perl's encoding; use slurp_raw

[Alex Muntada]

Hi Dominique,

> Dear debian-perl team colleagues, do you foresee any problem
> with this change?

None that I'm aware of... +1 to YAML::XS

Cheers!
Alex

--
  ⢀⣴⠾⠻⢶⣦⠀
  ⣾⠁⢠⠒⠀⣿⡁   Alex Muntada 
  ⢿⡄⠘⠷⠚⠋   Debian Developer  log.alexm.org
  ⠈⠳⣄



signature.asc
Description: PGP signature

Bug#880368: YAML::XS::Load expects utf8 octets, not perl's encoding; use slurp_raw

[gregor herrmann]

On Thu, 19 Dec 2019 10:18:12 +0100, Dominique Dumont wrote:

> Since the security issue related to YAML::XS has been fixed (provided 
> $YAML::XS::LoadBlessed is set to 0, which is not a problem for cme), I see no 
> issue to switch libconfig-model-dpkg-perl to YAML::XS instead of YAML::Tiny.
> 
> Dear debian-perl team colleagues, do you foresee any problem with this change 
> ?

No, sounds good.
(And we're using YAML::XS in other places as well, so at least for us
it's not even a new dependency.)


Cheers,
gregor

-- 
 .''`.  https://info.comodo.priv.at -- Debian Developer https://www.debian.org
 : :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D  85FA BB3A 6801 8649 AA06
 `. `'  Member VIBE!AT & SPI Inc. -- Supporter Free Software Foundation Europe
   `-   NP: Red Hot Chili Peppers: Road Trippin'


signature.asc
Description: Digital Signature

Bug#880368: YAML::XS::Load expects utf8 octets, not perl's encoding; use slurp_raw

[Dominique Dumont]

On Thursday, 19 December 2019 10:06:28 CET you wrote:
> However, I was unable to make pyyaml to generate the YAML format
> YAML::Tiny is always able to read, and apparently ruamel (judging by the
> code) has the same issue.

ok, I understand which instance of YAML::Tiny is causing trouble.

Since the security issue related to YAML::XS has been fixed (provided 
$YAML::XS::LoadBlessed is set to 0, which is not a problem for cme), I see no 
issue to switch libconfig-model-dpkg-perl to YAML::XS instead of YAML::Tiny.

Dear debian-perl team colleagues, do you foresee any problem with this change 
?

All the best

Dod

Bug#880368: YAML::XS::Load expects utf8 octets, not perl's encoding; use slurp_raw

[Andrej Shadura]

On 15/12/2019 19:30, Dominique Dumont wrote:
> On Fri, 13 Dec 2019 14:23:46 +0100 Andrej Shadura 
>  wrote:
>> As a temporary workaround, I patched the locally used version to use
>> YAML::XS, but as I see you won?t accept this patch upstream. Is there a
>> solution that would satisfy both conditions of how having security
>> issues and supporting proper YAML? 
> 
> YAML::XS security issues has been fixed upstream. Now 
> Config::Model::Backend::Yaml uses YAML:XS
> 
> That said, YAML input and output is used in several places in libconfig-model-
> dpkg-perl. I fail to see your use case which involves debian/copyright in 
> YAML 
> format.
> 
> Could you provide more details on your use case?

In Apertis, we use scan-copyrights to verify the licenses for updates
coming from Debian are under licenses we know we want, and that there
are no new files with unclear terms. For that, we ship a YAML file under
debian/apertis/ in the format of fill-copyright-blanks and a
gitignore-formatted whitelist file. We also tell scan-copyrights to scan
all files, not just whitelisted extensions. For this to work, we read
our own config files, the config files in the normal scan-copyrights
location, and parse debian/copyright to determine the license of
debian/, and then merge them so that scan-copyrights uses the merged
configuration.

However, I was unable to make pyyaml to generate the YAML format
YAML::Tiny is always able to read, and apparently ruamel (judging by the
code) has the same issue.

-- 
Cheers,
  Andrej

Bug#880368: YAML::XS::Load expects utf8 octets, not perl's encoding; use slurp_raw

[Dominique Dumont]

On Fri, 13 Dec 2019 14:23:46 +0100 Andrej Shadura 
 wrote:
> As a temporary workaround, I patched the locally used version to use
> YAML::XS, but as I see you won?t accept this patch upstream. Is there a
> solution that would satisfy both conditions of how having security
> issues and supporting proper YAML? 

YAML::XS security issues has been fixed upstream. Now 
Config::Model::Backend::Yaml uses YAML:XS

That said, YAML input and output is used in several places in libconfig-model-
dpkg-perl. I fail to see your use case which involves debian/copyright in YAML 
format.

Could you provide more details on your use case ?

> By the way, what are those security
> issues and how serious and relevant to scan-copyrights are they?

YAML specification allows serialisation of Perl object, which means its 
destructor is invoked when this object is destroyed. Since the yaml data that 
may contain a Perl object may comes from untrusted package source, this was a 
security issue (albeit quite far fetched).

Anyway, YAML::XS now has a switch to disable object creation when loading 
YAML. See https://github.com/ingydotnet/yaml-libyaml-pm/issues/45 for more 
details.

Bug#880368: YAML::XS::Load expects utf8 octets, not perl's encoding; use slurp_raw

[Andrej Shadura]

On Sun, 05 Nov 2017 18:32:48 +0100 Dominique Dumont  wrote:
> On Monday, 30 October 2017 15:27:32 CET you wrote:
> > YAML::XS::Load (and *hopefully* the other implementations of
> > YAML::Any::Load?) expect utf8 octets on input, not perl's internal
> > encoding.
> 
> Uh ? I thought I had gotten rid of YAML::Any... Well, after checking, it 
> turns 
> out that I've updated Config;:Model::Backend::Yaml, but I forgot to update 
> Dpkg::Scanner.
> 
> Anyway, using YAML::Any has several problems:
> - it's deprecated
> - it may load YAML or YAML::XS which have some security issues [1]
> 
> > Thus, slurp_raw should be used instead of slurp_utf8. [Though really,
> > YAML::XS::Load should probably do the right thing if is_utf8 is on,
> > anyway.]
> 
> Unfortunately, the strings returned by YAML::XS is not tagged as utf-8, which 
> leads to writing mojibake when cme is used to update debian/copyright.
>
> Given the security issues of YAML and YAML::XS, I'm not going to tweak the 
> structure returned by YAML::XS to fix the utf8 flag of each scalar contained 
> the structure (and may be all hash keys ..)
> 
> Instead, I'm going to replace YAML::Any with YAML::Tiny (which is more than 
> enough in this case).

Unfortunately, YAML::Tiny disallows some valid YAML markup, in
particular what pyyaml generates by default and which is very difficult
to change without in-depth hacking of it:

".*":
  "license": |-
GPL-2
"debian/":
  "copyright": "A B \n B C \n C\
\ D \n D E \n E F\
\ \n F G \n G H "
  "license": |-
GPL-2+

As a temporary workaround, I patched the locally used version to use
YAML::XS, but as I see you won’t accept this patch upstream. Is there a
solution that would satisfy both conditions of how having security
issues and supporting proper YAML? By the way, what are those security
issues and how serious and relevant to scan-copyrights are they?

> Thanks for the report . This helps me improve dpkg model for cme (and led to 
> the release of Config::Model::Tester 3.003 which did not handle utf-8 
> correctly while checking file content).


-- 
Cheers,
  Andrej

Bug#880368: YAML::XS::Load expects utf8 octets, not perl's encoding; use slurp_raw

[Don Armstrong]

On Sun, 05 Nov 2017, Dominique Dumont wrote:
> Anyway, using YAML::Any has several problems:
> - it's deprecated
> - it may load YAML or YAML::XS which have some security issues [1]

Heh; that's a pretty nice bug.

> Instead, I'm going to replace YAML::Any with YAML::Tiny (which is more than 
> enough in this case).

Sounds great; any solution is good for me. [I just want to be able to
write out UTF-8 names of copyright holders.]

> Thanks for the report . This helps me improve dpkg model for cme (and
> led to the release of Config::Model::Tester 3.003 which did not handle
> utf-8 correctly while checking file content).

No problem! Thanks for maintaining this. [It made updating
debian/copyright in autorandr much, much easier.]

-- 
Don Armstrong  https://www.donarmstrong.com

The game of science is, in principle, without end. He who decides one
day that scientific statements do not call for any further test, and
that they can be regarded as finally verified, retires from the game.
 -- Sir Karl Popper _The Logic of Scientific Discovery_ §11

Bug#880368: YAML::XS::Load expects utf8 octets, not perl's encoding; use slurp_raw

[Dominique Dumont]

On Monday, 30 October 2017 15:27:32 CET you wrote:
> YAML::XS::Load (and *hopefully* the other implementations of
> YAML::Any::Load?) expect utf8 octets on input, not perl's internal
> encoding.

Uh ? I thought I had gotten rid of YAML::Any... Well, after checking, it turns 
out that I've updated Config;:Model::Backend::Yaml, but I forgot to update 
Dpkg::Scanner.

Anyway, using YAML::Any has several problems:
- it's deprecated
- it may load YAML or YAML::XS which have some security issues [1]

> Thus, slurp_raw should be used instead of slurp_utf8. [Though really,
> YAML::XS::Load should probably do the right thing if is_utf8 is on,
> anyway.]

Unfortunately, the strings returned by YAML::XS is not tagged as utf-8, which 
leads to writing mojibake when cme is used to update debian/copyright.

Given the security issues of YAML and YAML::XS, I'm not going to tweak the 
structure returned by YAML::XS to fix the utf8 flag of each scalar contained 
the structure (and may be all hash keys ..)

Instead, I'm going to replace YAML::Any with YAML::Tiny (which is more than 
enough in this case).

Thanks for the report . This helps me improve dpkg model for cme (and led to 
the release of Config::Model::Tester 3.003 which did not handle utf-8 
correctly while checking file content).

All the best

[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=862373

-- 
 https://github.com/dod38fr/   -o- http://search.cpan.org/~ddumont/
http://ddumont.wordpress.com/  -o-   irc: dod at irc.debian.org

Bug#880368: YAML::XS::Load expects utf8 octets, not perl's encoding; use slurp_raw

[Don Armstrong]

Package: libconfig-model-dpkg-perl
Severity: normal
Version: 2.101
Tag: patch

YAML::XS::Load (and *hopefully* the other implementations of
YAML::Any::Load?) expect utf8 octets on input, not perl's internal
encoding.

Thus, slurp_raw should be used instead of slurp_utf8. [Though really,
YAML::XS::Load should probably do the right thing if is_utf8 is on,
anyway.]


-- 
Don Armstrong  https://www.donarmstrong.com

I will not make any deals with you. I've resigned. I will not be
pushed, filed, stamped, indexed, briefed, debriefed or numbered. My
life is my own. I resign.
 -- Patrick McGoohan as Number 6 in "The Prisoner"
From 778867c1a9c9ff80ad2ba272845397c39e4030e7 Mon Sep 17 00:00:00 2001
From: Don Armstrong 
Date: Mon, 30 Oct 2017 15:24:57 -0700
Subject: [PATCH] use slurp_raw for Load $yaml to get raw UTF8 octets

 YAML::XS::Load needs utf8 octets on input, not perl's internal encoding.
---
 lib/Dpkg/Copyright/Scanner.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/Dpkg/Copyright/Scanner.pm b/lib/Dpkg/Copyright/Scanner.pm
index b6e07be9..8db18d37 100644
--- a/lib/Dpkg/Copyright/Scanner.pm
+++ b/lib/Dpkg/Copyright/Scanner.pm
@@ -177,7 +177,7 @@ sub scan_files ( %args ) {
 my $scan_patterns = $debian->child("copyright-scan-patterns.yml");
 
 if ($debian->is_dir and $scan_patterns->is_file) {
-my $yaml = $scan_patterns->slurp_utf8;
+my $yaml = $scan_patterns->slurp_raw;
 # { check => { suffixes => [ js pl ], pattern => '[A-Z]*'}, ignore => { suffixes => [ jpg png ], pattern => 'foo.*'} }
 $scan_data = Load $yaml;
 }
@@ -613,7 +613,7 @@ sub __load_fill_blank_data ($current_dir) {
 
 warn "Note: loading @fills fixes" if @fills and not $quiet;
 foreach my $file ( @fills) {
-my $yaml = $file->slurp_utf8;
+my $yaml = $file->slurp_raw;
 my $data = Load $yaml;
 foreach my $path (sort keys %$data) {
 if ($fill_blanks{$path}) {
-- 
2.15.0.rc1