Skip to main content ↑ ↓
Topic: tint2 and tint2conf craash after last update (Read 1082 times) previous topic - next topic
0 Members and 5 Guests are viewing this topic.

Re: tint2 and tint2conf craash after last update

Reply #15 –
I would bet it's because Artix was built with fortify source 3 and Arch was built with fortify source 2 even though their devtools has fortify source 3.

Re: tint2 and tint2conf craash after last update

Reply #16 –
Is there some test I can try? I'm just building with makepkg and know nothing of fortify?

EDIT: I used arch check sec and see these results

Code: [Select]
$ checksec --file=/bin/tint2
RELRO           STACK CANARY      NX            PIE             RPATH      RUNPATH	Symbols		FORTIFY	Fortified	Fortifiable	FILE
Full RELRO      Canary found      NX enabled    PIE enabled     No RPATH   No RUNPATH   No Symbols	No	4		12		/bin/tint2
$ checksec --file=artix-tint2
RELRO           STACK CANARY      NX            PIE             RPATH      RUNPATH	Symbols		FORTIFY	Fortified	Fortifiable	FILE
Full RELRO      Canary found      NX enabled    PIE enabled     No RPATH   No RUNPATH   No Symbols	No	5		13		artix-tint2

tint2 is my self built binary and artix-tint2 is the one from the artix package

EDIT: FWIW the arch tint2 gives this
Code: [Select]
$ checksec --file=/bin/tint2
RELRO           STACK CANARY      NX            PIE             RPATH      RUNPATH	Symbols		FORTIFY	Fortified	Fortifiable	FILE
Full RELRO      Canary found      NX enabled    PIE enabled     No RPATH   No RUNPATH   No Symbols	No	4		12		/bin/tint2

Re: tint2 and tint2conf craash after last update

Reply #17 –
You can export it in the PKGBUIILD. e.g. (excuse the spaces/tabs mismatch)

Code: [Select]
# Maintainer: Cory Sanin <[email protected]>
# Contributor: Alexander F. Rødseth <[email protected]>
# Contributor: Robin Candau <[email protected]>
# Contributor: Blue Peppers <[email protected]>
# Contributor: Stefan Husmann <[email protected]>
# Contributor: Yannick LM <[email protected]>

pkgname=tint2
pkgver=17.0.2
pkgrel=4
pkgdesc='Basic, good-looking task manager for WMs'
arch=(x86_64)
url='https://gitlab.com/o9000/tint2'
license=(GPL-2.0-only)
depends=(gtk3 imlib2 startup-notification)
makedepends=(cmake git ninja setconf)
source=("git+${url}.git#tag=${pkgver}"
        fix_segfault.patch)
sha256sums=('60dcde15ac09508daffe59b9c35244fee771f66ee989193f37d81c823fc643da'
            'b7cd2936bb807478bbb356b96879dedbbfc464ed2f930f426a0123e39884f78f')

prepare() {
	setconf "${pkgname}/get_version.sh" VERSION="${pkgver}"
	# Patch to fix segfault issue when opening some apps like conky, mpv or steam
	# See https://gitlab.archlinux.org/archlinux/packaging/packages/tint2/-/issues/1
	cd "${pkgname}"
	patch -Np1 < "${srcdir}/fix_segfault.patch"
}

build() {
        export CFLAGS="${CFLAGS/_FORTIFY_SOURCE=3/_FORTIFY_SOURCE=2}"
	mkdir -p build
	cd build
	cmake ../"${pkgname}" \
	  -D CMAKE_INSTALL_PREFIX=/usr \
	  -D ENABLE_TINT2CONF=1 \
	  -G Ninja
	ninja
}

package() {
	DESTDIR="${pkgdir}" ninja -C build install
}

Re: tint2 and tint2conf craash after last update

Reply #18 –
Quote from: Dudemanguy – on
You can export it in the PKGBUIILD. e.g. (excuse the spaces/tabs mismatch)
.........
Code: [Select]
build() {
        export CFLAGS="${CFLAGS/_FORTIFY_SOURCE=3/_FORTIFY_SOURCE=2}"
........ja -C build install
}
Well done Dudemanguy, your suggestion works. For some reason your substitution using bashism did not work for me so I  used

export CFLAGS="$(echo ${CFLAGS} | sed -e's/_FORTIFY_SOURCE=2/_FORTIFY_SOURCE=3')"

instead. I checked that  CFLAGS did change to =3 and with that local build installed the buffer overflow is present ie using geany.desktop causes tint2 to crash immediately and if I comment that then launching tint2conf starts and crashes the app  with tint2 surviving. So Mr Homes your deductions were right on the mark  8)  ;D

EDIT: sorry for confusion; your code is about forcing the use of _FORTIFY_SOURCE=2 starting from _FORTIFY_SOURCE=3. On my system makepkg.conf seems to have _FORTIFY_SOURCE=2 and builds seem not to error. So I tried _FORTIFY_SOURCE=3 and I see errors.. Using gdb I see that the crash caused by geany.desktop involves a sprintf somewhere in the launcher code. I need to get symbols involved somehow.

Re: tint2 and tint2conf craash after last update

Reply #19 –
After using gdb with more symbols I see the crash with launcher_item=geany.desktop as

Code: [Select]
#5  0x00007ffff72ed75b in __GI___fortify_fail (msg=msg@entry=0x7ffff7376148 "buffer overflow detected") at fortify_fail.c:24
#6  0x00007ffff72ed106 in __GI___chk_fail () at chk_fail.c:28
#7  0x00007ffff72ee965 in ___snprintf_chk
    (s=s@entry=0x5555555f5106 "%", maxlen=maxlen@entry=118, flag=flag@entry=2, slen=slen@entry=112, format=format@entry=0x5555555a267f "%c%c")
    at snprintf_chk.c:29
#8  0x000055555557ca7c in snprintf (__fmt=0x5555555a267f "%c%c", __n=118, __s=0x5555555f5106 "%") at /usr/include/bits/stdio2.h:54
#9  expand_exec (entry=entry@entry=0x7fffffffde10, path=0x5555555eef00 "geany.desktop")
    at /home/robin/devel/tint2/src/tint2/src/launcher/apps-common.c:106
#10 0x000055555557ceb7 in read_desktop_file_full_path
    (path=path@entry=0x5555555d4840 "/usr/share/applications/geany.desktop", entry=entry@entry=0x7fffffffde10)
    at /home/robin/devel/tint2/src/tint2/src/launcher/apps-common.c:219

The code at line apps-common.c:106 looks like
Code: [Select]
                } else if (*p == 'f' || *p == 'F') {
                    snprintf(q, buf_size, "%c%c", '%', *p);
                    q += 2;
                    buf_size -= 2;
                    q--; // To balance the q++ in the for
                } else {......

I can try eliminating the check by using the obvious direct copy character by character.

Re: tint2 and tint2conf craash after last update

Reply #20 –
I am unsure what causes the actual FORTIFY failure at apps-common line 106. My guess is that because the pointer q has no obvious length the check is being overcautious.
I replace the snprintf version with a more explicit code ie

Code: [Select]
                } else if (*p == 'f' || *p == 'F') {
                    //snprintf(q, buf_size, "%c%c", '%', *p);
                    if (q+2 >= exec2+buf_size ){
                        fprintf(stderr,"*** buffer overflow detected at %s:%04d ***\n", __FILE__, __LINE__);
                        abort();
                    }
					q[0] = '%';
					q[1] = *p;
                    q += 2;
                    buf_size -= 2;
                    q--; // To balance the q++ in the for
                } else {
and I then see no errors with _FORTIFY_SOURCE=3 neither the geany.desktop or tint2conf.desktop seem to cause me problems. I can probably make a patch, but where do I send it?

Re: tint2 and tint2conf craash after last update

Reply #21 –
Since there's no official tint2 issue tracker maybe just post the patch here. Thanks for the job :-)

Re: tint2 and tint2conf craash after last update

Reply #22 –
here you are; this is only needed for _FORTIFY_SOURCE=3

Code: [Select]
diff --git a/PKGBUILD b/PKGBUILD
index 76552c2..05e01be 100644
--- a/PKGBUILD
+++ b/PKGBUILD
@@ -15,9 +15,12 @@ license=(GPL-2.0-only)
 depends=(gtk3 imlib2 startup-notification)
 makedepends=(cmake git ninja setconf)
 source=("git+${url}.git#tag=${pkgver}"
-        fix_segfault.patch)
+        fix_segfault.patch
+        fix_overflow.patch
+        )
 sha256sums=('60dcde15ac09508daffe59b9c35244fee771f66ee989193f37d81c823fc643da'
-            'b7cd2936bb807478bbb356b96879dedbbfc464ed2f930f426a0123e39884f78f')
+            'b7cd2936bb807478bbb356b96879dedbbfc464ed2f930f426a0123e39884f78f'
+            '9226b30dcec17cf03ac0873d90e5df3619c7e44efc2b527f4cec0ada49ac41cc')
 
 prepare() {
 	setconf "${pkgname}/get_version.sh" VERSION="${pkgver}"
@@ -25,6 +28,7 @@ prepare() {
 	# See https://gitlab.archlinux.org/archlinux/packaging/packages/tint2/-/issues/1
 	cd "${pkgname}"
 	patch -Np1 < "${srcdir}/fix_segfault.patch"
+	patch -Np1 < "${srcdir}/fix_overflow.patch"
 }
 
 build() {

Code: [Select]
$ cat fix_overflow.patch 
--- a/src/launcher/apps-common.c
+++ b/src/launcher/apps-common.c
@@ -104,6 +104,10 @@
                     q--; // To balance the q++ in the for
                 } else if (*p == 'f' || *p == 'F') {
-                    snprintf(q, buf_size, "%c%c", '%', *p);
-                    q += 2;
+                    if (q+2 >= exec2+buf_size ){
+                        fprintf(stderr,"*** buffer overflow detected at %s:%04d ***\n", __FILE__, __LINE__);
+                        abort();
+                    }
+                    *q++ = '%';
+                    *q++ = *p;
                     buf_size -= 2;
                     q--; // To balance the q++ in the for

Re: tint2 and tint2conf craash after last update

Reply #23 –
For anyone interested here is my git diff HEAD against the Artix tint2 gitea; probably would be wise to run updpkgsums if you copy from here.

Code: [Select]
diff --git a/PKGBUILD b/PKGBUILD
index 76552c2..05e01be 100644
--- a/PKGBUILD
+++ b/PKGBUILD
@@ -15,9 +15,12 @@ license=(GPL-2.0-only)
 depends=(gtk3 imlib2 startup-notification)
 makedepends=(cmake git ninja setconf)
 source=("git+${url}.git#tag=${pkgver}"
-        fix_segfault.patch)
+        fix_segfault.patch
+        fix_overflow.patch
+        )
 sha256sums=('60dcde15ac09508daffe59b9c35244fee771f66ee989193f37d81c823fc643da'
-            'b7cd2936bb807478bbb356b96879dedbbfc464ed2f930f426a0123e39884f78f')
+            'b7cd2936bb807478bbb356b96879dedbbfc464ed2f930f426a0123e39884f78f'
+            '9226b30dcec17cf03ac0873d90e5df3619c7e44efc2b527f4cec0ada49ac41cc')
 
 prepare() {
 	setconf "${pkgname}/get_version.sh" VERSION="${pkgver}"
@@ -25,6 +28,7 @@ prepare() {
 	# See https://gitlab.archlinux.org/archlinux/packaging/packages/tint2/-/issues/1
 	cd "${pkgname}"
 	patch -Np1 < "${srcdir}/fix_segfault.patch"
+	patch -Np1 < "${srcdir}/fix_overflow.patch"
 }
 
 build() {
diff --git a/fix_overflow.patch b/fix_overflow.patch
new file mode 100644
index 0000000..5807bea
--- /dev/null
+++ b/fix_overflow.patch
@@ -0,0 +1,15 @@
+--- a/src/launcher/apps-common.c
++++ b/src/launcher/apps-common.c
+@@ -104,6 +104,10 @@
+                     q--; // To balance the q++ in the for
+                 } else if (*p == 'f' || *p == 'F') {
+-                    snprintf(q, buf_size, "%c%c", '%', *p);
+-                    q += 2;
++                    if (q+2 >= exec2+buf_size ){
++                        fprintf(stderr,"*** buffer overflow detected at %s:%04d ***\n", __FILE__, __LINE__);
++                        abort();
++                    }
++                    *q++ = '%';
++                    *q++ = *p;
+                     buf_size -= 2;
+                     q--; // To balance the q++ in the for

Re: tint2 and tint2conf craash after last update

Reply #24 –
It's a shame that there's no proper artix bugtracker. Maybe an email to current tint2 maintainer?

Re: tint2 and tint2conf craash after last update

Reply #25 –
Quote from: Ambie – on
It's a shame that there's no proper artix bugtracker. Maybe an email to current tint2 maintainer?
Ambie I notice that at first you tried a local build that failed. does your /etc/makepkg.conf CFLAGS contain

-D_FORTIFY_SOURCE=2 or -D_FORTIFY_SOURCE=3

The tint2 maintainer is corysanin and he commented in this thread. If you have -D_FORTIFY_SOURCE=3 and the patch fixes the local build for you then perhaps you can drop him an email to say that this patch fixes an issue for you. It does for me when I use  -D_FORTIFY_SOURCE=3
 

Re: tint2 and tint2conf craash after last update

Reply #26 –
I pushed the patched version to gremlins. Let me know if it works.