cdbs: control auto-build messes with long descriptions

[Debian Bug Importer]

Automatically imported from Debian bug report #301607 http://bugs.debian.org/301607

[Jonas Smedegaard (dr)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

severity 301607 critical
thanks

Hi again,

I just discovered that double quotes in long descripions get stripped
too, when the auto-update feature is enabled.

That specific one is solved with the attached patch.

But what worries me most, though, is that the complete control.in file
is interpreted as a shell script. What if someone (in good faith) had a
small code example included in a long description like this:

 Don't use this package to do "my_package > /etc/passwd" directly.

That wouldn't just break - it would do funny things to your build
environment!!! Thank goodness we all always use fakeroot - right?

I dare raise the severity of this bug. Yes, I am aware that
DEB_AUTO_UPDATE_DEBIAN_CONTROL is an experimental feature, but still it
shouldn't be able to cause surprises as fatal as these.

I very strongly suggest not to trust the complete debian/control file to
be interpreted sanely as a shell script. Let's figure out a way to only
mess with explicitly selected lines of the file instead.

 - Jonas

- --
* Jonas Smedegaard - idealist og Internet-arkitekt
* Tlf.: [phone] Website: http://dr.jones.dk/

 - Enden er nær: http://www.shibumi.org/eoti.htm
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFCT6j7n7DbMsAkQLgRAvUjAJwI1q/Ew6uoLAwCIFc0IA4PI6foGACgilia
iTyfEDwktlTnNtA/1aytB+c=
=IJiJ
-----END PGP SIGNATURE-----

[Debian Bug Importer (debzilla)]

Automatically imported from Debian bug report #301607 http://bugs.debian.org/301607

[Debian Bug Importer (debzilla)]

Message-Id: <email address hidden>
Date: Sun, 27 Mar 2005 05:12:52 +0200
From: Jonas Smedegaard <email address hidden>
To: Debian Bug Tracking System <email address hidden>
Subject: cdbs: control auto-build messes with long descriptions

Package: cdbs
Version: 0.4.27-3
Severity: important

The package cstream contains the following in the long description:

  - Accepts 'k', 'm' and 'g' character after number for kilo, mega,
    giga bytes for overall data size limit.

When enabling DEB_AUTO_UPDATE_DEBIAN_CONTROL building breaks due to
the generated control file containing this:

  - Accepts 'k', 'm' and 'g' character after number for kilo, mega,
echo giga bytes for overall data size limit.

My guess is cdbs wrongly assumes lines ending with comma should be
messed with, but I can't locate the exact error myself.

Tagging as important as I suspect there's risk of more harm if the
auto-update sed routine does not mess only with Build-depends line.

 - Jonas

-- System Information:
Debian Release: 3.1
  APT prefers unstable
  APT policy: (500, 'unstable'), (500, 'testing')
Architecture: powerpc (ppc)
Kernel: Linux 2.6.12-rc1-mm3
Locale: LANG=da_DK, LC_CTYPE=da_DK (charmap=ISO-8859-1)

-- no debconf information

[Debian Bug Importer (debzilla)]

Message-ID: <email address hidden>
Date: Sun, 03 Apr 2005 10:27:40 +0200
From: Jonas Smedegaard <email address hidden>
To: <email address hidden>, <email address hidden>
Subject: Double quotes gets stripped too!

--------------030804000009060403020103
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

severity 301607 critical
thanks

Hi again,

I just discovered that double quotes in long descripions get stripped
too, when the auto-update feature is enabled.

That specific one is solved with the attached patch.

But what worries me most, though, is that the complete control.in file
is interpreted as a shell script. What if someone (in good faith) had a
small code example included in a long description like this:

 Don't use this package to do "my_package > /etc/passwd" directly.

That wouldn't just break - it would do funny things to your build
environment!!! Thank goodness we all always use fakeroot - right?

I dare raise the severity of this bug. Yes, I am aware that
DEB_AUTO_UPDATE_DEBIAN_CONTROL is an experimental feature, but still it
shouldn't be able to cause surprises as fatal as these.

I very strongly suggest not to trust the complete debian/control file to
be interpreted sanely as a shell script. Let's figure out a way to only
mess with explicitly selected lines of the file instead.

 - Jonas

- --
* Jonas Smedegaard - idealist og Internet-arkitekt
* Tlf.: [phone] Website: http://dr.jones.dk/

 - Enden er n=C3=A6r: http://www.shibumi.org/eoti.htm
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFCT6j7n7DbMsAkQLgRAvUjAJwI1q/Ew6uoLAwCIFc0IA4PI6foGACgilia
iTyfEDwktlTnNtA/1aytB+c=3D
=3DIJiJ
-----END PGP SIGNATURE-----

--------------030804000009060403020103
Content-Type: text/x-patch;
 name="buildcore.mk.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="buildcore.mk.diff"

--- /usr/share/cdbs/1/rules/buildcore.mk 2005-03-10 13:35:06.000000000 +0100
+++ buildcore.mk 2005-04-03 10:07:28.000000000 +0200
@@ -211,6 +211,7 @@
   -e "s/\[cpu: \([^]]*\)\]/\[\`type-handling \\\\\`echo \1 | tr ' ' ','\\\\\` any\`\]/g" \
   -e "s/\[system: \([^]]*\)\]/\[\`type-handling any \\\\\`echo \1 | tr ' ' ','\\\\\`\`\]/g" \
   \
+ -e "s/\"/\\\\\"/g" \
   -e "s/^/echo \"/g" \
   -e "s/\\$$/\\\\$$/g" \
   -e "s/$$/\"/g" \

--------------030804000009060403020103--

[Matt Zimmerman (mdz)]

Jeff, is this something to worry about for Hoary or no?

[Jeff Bailey (jbailey)]

(In reply to comment #3)
> Jeff, is this something to worry about for Hoary or no?

I think no, but probably something to take care of before the next big merge.

Marking as 5.10, increasing severity to critical.

[Steve Langasek (vorlon)]

clone 301607 -1
reassign -1 cstream
severity -1 serious
retitle -1 cstream: debian/rules clean modifies the source package
severity 301607 important
thanks

I don't consider this a release-critical bug in cdbs because packages
shouldn't be using DEB_AUTO_UPDATE_DEBIAN_CONTROL *anyway*: packages that do
so are in violation of policy section 4.8, because it means
./debian/rules build clean is no longer idempotent when run using differing
versions of cdbs.

As a result, while I consider DEB_AUTO_UPDATE_DEBIAN_CONTROL a gross
misfeature of cdbs that ought to be removed, it is the responsibility of
other package maintainers to not use it for packages uploaded to Debian --
it is not the fault of the cdbs maintainers that you're using an optional
feature of cdbs that violates policy.

Regards,
--
Steve Langasek
postmodern programmer

[Debian Bug Importer (debzilla)]

Message-ID: <email address hidden>
Date: Sat, 16 Apr 2005 03:30:14 -0700
From: Steve Langasek <email address hidden>
To: <email address hidden>
Subject: Re: cdbs: control auto-build messes with long descriptions

--sDKAb4OeUBrWWL6P
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

clone 301607 -1
reassign -1 cstream
severity -1 serious
retitle -1 cstream: debian/rules clean modifies the source package
severity 301607 important
thanks

I don't consider this a release-critical bug in cdbs because packages
shouldn't be using DEB_AUTO_UPDATE_DEBIAN_CONTROL *anyway*: packages that do
so are in violation of policy section 4.8, because it means
=2E/debian/rules build clean is no longer idempotent when run using differi=
ng
versions of cdbs.

As a result, while I consider DEB_AUTO_UPDATE_DEBIAN_CONTROL a gross
misfeature of cdbs that ought to be removed, it is the responsibility of
other package maintainers to not use it for packages uploaded to Debian --
it is not the fault of the cdbs maintainers that you're using an optional
feature of cdbs that violates policy.

Regards,
--=20
Steve Langasek
postmodern programmer

--sDKAb4OeUBrWWL6P
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: Digital signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (GNU/Linux)

iD8DBQFCYOkxKN6ufymYLloRAgMJAJ92l0shTszVFz+zW1JRfAVtR/qiMACdGaLv
Dd9zWnlqTmyxYpG+WOE7QIc=
=30Zw
-----END PGP SIGNATURE-----

--sDKAb4OeUBrWWL6P--

[Robert Millan (rmh)]

On Sat, Apr 16, 2005 at 03:30:14AM -0700, Steve Langasek wrote:
> clone 301607 -1
> reassign -1 cstream
> severity -1 serious
> retitle -1 cstream: debian/rules clean modifies the source package
> severity 301607 important
> thanks
>
> I don't consider this a release-critical bug in cdbs because packages
> shouldn't be using DEB_AUTO_UPDATE_DEBIAN_CONTROL *anyway*: packages that do
> so are in violation of policy section 4.8, because it means
> ./debian/rules build clean is no longer idempotent when run using differing
> versions of cdbs.

That's for a very particular interpretation of "idempotency" (that when the
environment changes, results must be the same).

If dpkg didn't require debian/control to exist during the clean target, we
could generate it somewhere else and remove it during clean (just like any
other temporary file) and fully idempotency would be archieved.

If you're concerned about that, you might want to read bug #293489 and try to
convince the dpkg maintainers about my proposed change. I'd certainly
appreciate it if that happened.

--
 .''`. Proudly running Debian GNU/kFreeBSD unstable/unreleased (on UFS2+S)
: :' :
`. `' http://www.debian.org/ports/kfreebsd-gnu
  `-

[Steve Langasek (vorlon)]

On Wed, May 18, 2005 at 12:42:10PM +0200, Robert Millan wrote:
> On Sat, Apr 16, 2005 at 03:30:14AM -0700, Steve Langasek wrote:
> > clone 301607 -1
> > reassign -1 cstream
> > severity -1 serious
> > retitle -1 cstream: debian/rules clean modifies the source package
> > severity 301607 important
> > thanks

> > I don't consider this a release-critical bug in cdbs because packages
> > shouldn't be using DEB_AUTO_UPDATE_DEBIAN_CONTROL *anyway*: packages that do
> > so are in violation of policy section 4.8, because it means
> > ./debian/rules build clean is no longer idempotent when run using differing
> > versions of cdbs.

> That's for a very particular interpretation of "idempotency" (that when the
> environment changes, results must be the same).

> If dpkg didn't require debian/control to exist during the clean target, we
> could generate it somewhere else and remove it during clean (just like any
> other temporary file) and fully idempotency would be archieved.

> If you're concerned about that, you might want to read bug #293489 and try to
> convince the dpkg maintainers about my proposed change. I'd certainly
> appreciate it if that happened.

No, I think those proposed changes are a terrible idea. debian/control
should not undergo functional changes as a result of a changing environment,
and anything that depends on such a feature is broken.

--
Steve Langasek
postmodern programmer

[Matt Zimmerman (mdz)]

What needs to be done about this bug for Breezy?

[Jeff Bailey (jbailey)]

(In reply to comment #6)
> What needs to be done about this bug for Breezy?

The basic quoting fix has been merged as part of the import of the debian
package, so the the reported problem is fixed. The observation that control
files shouldn't be trusted to a shell script, while reasonable, isn't that
troublesome. The maintainer is the one who's writing the file, and if (s)he
wanted to trojan the machine, debian/rules is a far simpler place to do it.

Lowering severity to normal, marking bug upstream. This is another nice
reminder that I should finish cdbs2.

Tks,
Jeff Bailey

[Debian Bug Importer (debzilla)]

Message-ID: <20050518104210.GA20020@aragorn>
Date: Wed, 18 May 2005 12:42:10 +0200
From: Robert Millan <email address hidden>
To: Steve Langasek <email address hidden>, <email address hidden>
Subject: Re: [Build-common-hackers] Bug#301607: cdbs: control auto-build messes with long
 descriptions

On Sat, Apr 16, 2005 at 03:30:14AM -0700, Steve Langasek wrote:
> clone 301607 -1
> reassign -1 cstream
> severity -1 serious
> retitle -1 cstream: debian/rules clean modifies the source package
> severity 301607 important
> thanks
>
> I don't consider this a release-critical bug in cdbs because packages
> shouldn't be using DEB_AUTO_UPDATE_DEBIAN_CONTROL *anyway*: packages that do
> so are in violation of policy section 4.8, because it means
> ./debian/rules build clean is no longer idempotent when run using differing
> versions of cdbs.

That's for a very particular interpretation of "idempotency" (that when the
environment changes, results must be the same).

If dpkg didn't require debian/control to exist during the clean target, we
could generate it somewhere else and remove it during clean (just like any
other temporary file) and fully idempotency would be archieved.

If you're concerned about that, you might want to read bug #293489 and try to
convince the dpkg maintainers about my proposed change. I'd certainly
appreciate it if that happened.

--
 .''`. Proudly running Debian GNU/kFreeBSD unstable/unreleased (on UFS2+S)
: :' :
`. `' http://www.debian.org/ports/kfreebsd-gnu
  `-

[Debian Bug Importer (debzilla)]

Message-ID: <email address hidden>
Date: Thu, 19 May 2005 07:49:15 -0700
From: Steve Langasek <email address hidden>
To: Robert Millan <email address hidden>
Cc: <email address hidden>
Subject: Re: [Build-common-hackers] Bug#301607: cdbs: control auto-build messes with long
 descriptions

--vqZEy/DEMZDTzjXG
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, May 18, 2005 at 12:42:10PM +0200, Robert Millan wrote:
> On Sat, Apr 16, 2005 at 03:30:14AM -0700, Steve Langasek wrote:
> > clone 301607 -1
> > reassign -1 cstream
> > severity -1 serious
> > retitle -1 cstream: debian/rules clean modifies the source package
> > severity 301607 important
> > thanks

> > I don't consider this a release-critical bug in cdbs because packages
> > shouldn't be using DEB_AUTO_UPDATE_DEBIAN_CONTROL *anyway*: packages th=
at do
> > so are in violation of policy section 4.8, because it means
> > ./debian/rules build clean is no longer idempotent when run using diffe=
ring
> > versions of cdbs.

> That's for a very particular interpretation of "idempotency" (that when t=
he
> environment changes, results must be the same).

> If dpkg didn't require debian/control to exist during the clean target, we
> could generate it somewhere else and remove it during clean (just like any
> other temporary file) and fully idempotency would be archieved.

> If you're concerned about that, you might want to read bug #293489 and tr=
y to
> convince the dpkg maintainers about my proposed change. I'd certainly
> appreciate it if that happened.

No, I think those proposed changes are a terrible idea. debian/control
should not undergo functional changes as a result of a changing environment,
and anything that depends on such a feature is broken.

--=20
Steve Langasek
postmodern programmer

--vqZEy/DEMZDTzjXG
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: Digital signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFCjKdqKN6ufymYLloRArNZAJ9TuCcTJbJFemMHay7pVBJgfX1gOACffWd8
HpXYIGzUNj6pzd+azVer1K4=
=Mb26
-----END PGP SIGNATURE-----

--vqZEy/DEMZDTzjXG--

[Matt Zimmerman (mdz)]

Too late for 5.10

[Jeff Bailey (jbailey)]

Unsetting milestone. No reason to target 5.10

[Peter Eisentraut (petere)]

Is this still applicable? I can't seem to reproduce it anymore with the
package you mentioned. Note that your proposed patch has been applied
in the meantime.

[Peter Eisentraut (petere)]

# Automatically generated email from bts, devscripts version 2.9.10
tags 301607 + unreproducible

[Peter Eisentraut (petere)]

It seems this has already been fixed.

[Carthik Sharma (carthik)]

Fix present in version 0.4.32ubuntu19 according to Debian BTS. Closing.