Bug 454980 (axel)

Lots of issues here. A first brief look:

  $ rpmlint axel-1.1-1.fc9.src.rpm 
  axel.src: W: invalid-license GPL
  axel.src: W: strange-permission axel-1.1.tar.gz 0775

  $ rpmlint ~/tmp/rpm/RPMS/axel-1.1-1.fc8.i386.rpm 
  axel.i386: W: invalid-license GPL
  axel.i386: W: non-standard-dir-in-usr etc

* According to the source files, the licence is: GPLv2+
https://fedoraproject.org/wiki/Licensing

Why is the config file not in /etc and not marked %config or
%config(noreplace) either? The manual page refers to /etc/axelrc!

In the build log:

./configure: line 43: program-prefix=: command not found
./configure: line 43: exec-prefix=/usr: No such file or directory

* The configure script is incompatible with the %configure macro.

* Fedora's global compiler flags are not used.

* The BuildRoot tag is truncated/damaged.

* The executable must not be stripped, because rpmbuild does that
when it creates the axel-debuginfo package.

* Why is internationalisation disabled?

Ankur, would you update your srpm?

If there is no objection may be I can continue maintenance this package?

SPEC: http://hubbitus.net.ru/rpm/Fedora9/axel/axel.spec
SRPM: http://hubbitus.net.ru/rpm/Fedora9/axel/axel-1.1-1.fc9.src.rpm

(In reply to comment #1)
All changes made by your comment (see changelog):

- Correct BuildRoot
- License changed to GPLv2+
- Add patch0 - axel-1.1-fedora-build.patch
- Regular file /usr/etc/axelrc changed to %config(noreplace) %{_sysconfdir}/axelrc
- Disable strip binaries (switch --strip=0 to configure script)
- Add internationalisation support:
	. Switch --i18n=1 to configure script
	. Add BR gettext
	. Add files for de and nl locales.

rpmlint is quiet.
And it is built in koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=761927

This is my 4th (in Fedora package review, not in packaging history at all)
package and I am looking for sponsor.

We will still wait for the reply from Ankur (usually one month is required
before switching submitter)

@all 
i am sorry for not being able to reply, this was my first try at packaging and well it was full of errors!!!! altough i updated my spec there are still some problem with the configure script how do i solve it???

@  Pavel Alexeev
Yes i have no problem if you maintain this package

(In reply to comment #6)
> @all 
> i am sorry for not being able to reply, this was my first try at packaging and
> well it was full of errors!!!! altough i updated my spec there are still some
> problem with the configure script how do i solve it???
> 
> @  Pavel Alexeev
> Yes i have no problem if you maintain this package

Umm.. would you want to withdraw your submittion or want Pavel to co-maintain this
package? If you want to continue this review request, would you upload your newest
srpm anyway?

> altough i updated my spec there are still some
> problem with the configure script how do i solve it???

Run the script directly instead of using the %configure macro.
As I pointed out, for this package, the custom configure script
is incompatible with the %configure macro.

If you wish, I agree to co-maintain package.
But I do not insist.

In any case
(In reply to comment #6)
> altough i updated my spec there are still some
> problem with the configure script how do i solve it???
you may see how I resolve this trouble and get my patch if you want.

> http://hubbitus.net.ru/rpm/Fedora9/axel/axel.spec

Don't use  %configure  but ./configure with proper arguments,
because this configure script is a custom one.


> %lang(de)	%{_datadir}/locale/de/LC_MESSAGES/%{name}.mo
> %lang(nl)	%{_datadir}/locale/nl/LC_MESSAGES/%{name}.mo

The proper way to include these files is the %find_lang macro:

  %find_lang %{name}

somewhere at end of %install plus a change in the %files section,

  %files -f %{name}.lang

and then don't include the .mo files manually.


> %{_mandir}/man1/axel.1.gz

Currently, it really is automatic .gz compression. Better use a
wildcard, so the package keeps building whenever the underlying
compression method changes:

  %{_mandir}/man1/axel.1*

Ankur Shrivastava, do you make any decision accordingly this package? Do you want continue working on this package? Or do you want co-maintain it?

(In reply to comment #10)
> Don't use  %configure  but ./configure with proper arguments,
> because this configure script is a custom one.
Custom? Koji build successfully on any architectures, so, this macros expanded. What is a reason why I should provide manually all this parameters like PREFIX and others standard?

> The proper way to include these files is the %find_lang macro:
Yes. I'm known it now.
Well, let them be so.

> 
> > %{_mandir}/man1/axel.1.gz
> 
> Currently, it really is automatic .gz compression. Better use a
> wildcard, so the package keeps building whenever the underlying
> compression method changes:
Ok.

http://hubbitus.net.ru/rpm/Fedora9/axel/axel-1.1-2.fc9.src.rpm
http://hubbitus.net.ru/rpm/Fedora9/axel/axel.spec

> ./configure 

See comment 1. One can argue that non-fatal errors like that are
acceptable, but they can be avoided.

(In reply to comment #12)
> See comment 1. One can argue that non-fatal errors like that are
> acceptable, but they can be avoided.
As you can see, my package hasn't this errors. http://koji.fedoraproject.org/koji/getfile?taskID=797541&name=build.log

Well, you patched the configure script as a work-around. ;)

[...]

The build.log points to sloppy strncpy/strncat usage. A buffer
overflow is triggered if the length of the constructed output
file path grows over 1024 characters:

text.c: In function 'main':
text.c:167: warning: ignoring return value of 'scanf', declared with attribute w
arn_unused_result
In function 'strncat',
    inlined from 'main' at text.c:255:
/usr/include/bits/string3.h:153: warning: call to __builtin___strncat_chk might 
overflow destination buffer

(In reply to comment #14)
> Well, you patched the configure script as a work-around. ;)
Yes, it is.

> The build.log points to sloppy strncpy/strncat usage. A buffer
> overflow is triggered if the length of the constructed output
> file path grows over 1024 characters:

I saw its warnings. But it is only warnings and it is related to upstream source code, not to rpm itself or packaging process or my patch of configure script.

It's a warning about a potential buffer overflow, that ought
to be examined by the package maintainer. The warning alone
is reason enough to say "Ooops, we don't want to release this".

I've reported above bug to upstream via email.

Insecure C string operations like that lead to vulnerabilities.
There are more in the code. No matter how likely it is to trigger
them, they ought to be fixed.

Initializing download: http://fedoraproject.org
*** buffer overflow detected ***: axel terminated
(gdb) bt
#0  0x00110416 in __kernel_vsyscall ()
#1  0x00482690 in raise () from /lib/libc.so.6
#2  0x00483f91 in abort () from /lib/libc.so.6
#3  0x004ba9eb in __libc_message () from /lib/libc.so.6
#4  0x00542b58 in __fortify_fail () from /lib/libc.so.6
#5  0x00541200 in __chk_fail () from /lib/libc.so.6
#6  0x00540918 in _IO_str_chk_overflow () from /lib/libc.so.6
#7  0x004bee8d in _IO_default_xsputn_internal () from /lib/libc.so.6
#8  0x00495fcf in vfprintf () from /lib/libc.so.6
#9  0x005409cd in __vsprintf_chk () from /lib/libc.so.6
#10 0x00540900 in __sprintf_chk () from /lib/libc.so.6
#11 0x0804eac4 in main (argc=Cannot access memory at address 0xb19
) at /usr/include/bits/stdio2.h:34
#12 0x0046f390 in __libc_start_main () from /lib/libc.so.6
#13 0x080491d1 in _start ()

>I've reported above bug to upstream via email.
Thank you, Michael Schwendt.

Please let us know when to receive a response.

2.0 seems released. Pavel, would you check if the issue
reported by Michael is resolved?

Eh, Michael Schwendt, make decision do not provide info from upstream? :(

Mamoru Tasaka, it seems all fixed in new 2.0 version.

In build log I see only one warning (except of several about unesed return values) on first glance:
In function 'strncat',
    inlined from 'main' at text.c:259:
/usr/include/bits/string3.h:153: warning: call to __builtin___strncat_chk might overflow destination buffer

But it is not axel trouble I thought.

http://koji.fedoraproject.org/koji/taskinfo?taskID=891915

Haven't got any reply from upstream.

My message went to "Wilmer van der Gaast <wilmer>"
on Sep 1st and also linked this ticket.

And it is the same bug as commented on before.

Michael Schwendt, may bebest solution is to fill bug to upstream ( http://alioth.debian.org/tracker/?atid=413085&group_id=100070&func=browse )?

Would you do that? Or I can do that.

I dunno whether I'm registered there. It's beyond my time to
file a bug in that tracker.

And indeed the code suffers from other buffer overflows caused
by the same programming mistake:
http://alioth.debian.org/tracker/?func=detail&atid=413085&aid=311178&group_id=100070

Meanwhile 2.2 is the latest release.

2.2? Ok, let it be 2.2.

http://koji.fedoraproject.org/koji/taskinfo?taskID=898668

*** Bug 469473 has been marked as a duplicate of this bug. ***

What is the status of this bug? Currently can we regard that Pavel is the submitter?

By the way:
To Pavel
Usually koji scratch build is deleted within about one week. If you are the
current submitter would you upload your srpm on the different URL?

Hello, Mamoru.

Sorry, off course I can provide other link - http://hubbitus.net.ru/rpm/Fedora9/axel/

I have no reason to believe that the author is interested in avoiding buffer overflow problems. My problem report (Sep 1st) and the forwarded copy (Oct 21st) are unanswered, too.

Meanwhile mentioned above bug http://alioth.debian.org/tracker/?func=detail&atid=413085&aid=311178&group_id=100070 of buffer owerflow in http.c was resolved and closed!

That's the other one I pointed to in comment 22 already

Off course this is bug pointed by you. I only what it was resolved and closed. May be you can recheck other? And if it was gone, we can continue review.

New version 2.3 released.

http://hubbitus.net.ru/rpm/Fedora9/axel/axel-2.3-1.fc9.src.rpm
http://hubbitus.net.ru/rpm/Fedora9/axel/axel.spec

$ rpmlint /home/qa/tmp/rpm/RPMS/axel-2.3-1.fc10.i386.rpm

axel.i386: E: incorrect-locale-subdir /usr/share/locale/zh_cn/LC_MESSAGES/axel.mo
1 packages and 0 specfiles checked; 1 errors, 0 warnings.

[...]

Build uses compiler flags that don't adhere to the Fedora Packaging Guidelines:
https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags

[...]

Buffer overflow warning is still printed. => I won't approve the package. Rationale given in earlier comments.

(In reply to comment #32)
> axel.i386: E: incorrect-locale-subdir
> /usr/share/locale/zh_cn/LC_MESSAGES/axel.mo
> 1 packages and 0 specfiles checked; 1 errors, 0 warnings.
Hmmm. It is generated by standard %find_lang macros. Where I wrong with it?


> Build uses compiler flags that don't adhere to the Fedora Packaging Guidelines:
> https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags
I do not agree with that. configure patched to use those flags. I assume there all right.


> Buffer overflow warning is still printed. => I won't approve the package.
> Rationale given in earlier comments.  
You are speak about using standard strncat C function:
/usr/include/bits/string3.h:153: warning: call to __builtin___strncat_chk might overflow destination buffer
???

Comment 14 starts with comments on the the buffer overflow/security issues.


> Hmmm. It is generated by standard %find_lang macros.

It isn't. The %find_lang macro only finds message/translation object files.


> configure patched to use those flags. I assume there all right.

Not true. Wrong assumption.

(In reply to comment #34)
> Comment 14 starts with comments on the the buffer overflow/security issues.
I file bug to upstream https://bugzilla.redhat.com/show_bug.cgi?id=454980#c14

> > Hmmm. It is generated by standard %find_lang macros.
> It isn't. The %find_lang macro only finds message/translation object files.
And generate file of list it. Aftre that it used also in standard way:
%files -f %{name}.lang

What there wrong?

> > configure patched to use those flags. I assume there all right.
> Not true. Wrong assumption.  
I had to disagree there.
In standard configure script it place this flags 'CFLAGS=-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -g -Os' into Makefile.settings.
But as you can see in build.log it compiled with standard Fedora flags, where only few upstrem defined flagw was reseeded:

CFLAGS='-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=athlon -fasynchronous-unwind-tables'
...
gcc -c axel.c -o axel.o -Wall -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=athlon -fasynchronous-unwind-tables -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -g -Os

Just for this:

(In reply to comment #33)
> (In reply to comment #32)
> > axel.i386: E: incorrect-locale-subdir
> > /usr/share/locale/zh_cn/LC_MESSAGES/axel.mo
> > 1 packages and 0 specfiles checked; 1 errors, 0 warnings.
> Hmmm. It is generated by standard %find_lang macros. Where I wrong with it?

Message catalogue .mo file for Chinese Simplified should be
installed under "zh_CN", not "zh_cn".

> But as you can see in build.log it compiled with
> standard Fedora flags, where only few upstrem defined
> flagw was reseeded:

Quoting the guidelines:

| Adding to and overriding or filtering parts of these
| flags is permitted if there's a good reason to do so;
| the rationale for doing so should be reviewed and
| documented in the specfile especially in the override
| and filter cases.

Additionally, shouldn't the default prefix /usr/local be changed to /usr ? Just calling configure with --prefix=/usr should suffice for that.

@Philipp: Run "rpm --eval %configure|less" in your favourite terminal.

@Michael Schwendt
Ok, I move Fedorian flags after autors, so, if dupes present, system wide will override. It is correct? Or I must use fully only system-defaults flags here?

(In reply to comment #36)
> Message catalogue .mo file for Chinese Simplified should be
> installed under "zh_CN", not "zh_cn".  
Ok. I assume it is author error. I correct that. Thank you.

@Philipp Macros %install provide explicit many default system flags, in this list also --prefix=/usr.

http://hubbitus.net.ru/rpm/Fedora9/axel/axel-2.3-2.fc9.src.rpm

Axel 2.4 has been release with the said bugs fixed.  Please file a bug at the axel bug tracker in case there is something else that you would like to see being addressed.

Ok, I built it: http://hubbitus.net.ru/rpm/Fedora9/axel/axel-2.4-3.fc9.src.rpm

hi,

I would like to maintain/co-maintain this package. 

here are the specs etc:

http://ankursinha.fedorapeople.org/axel/axel.spec

http://ankursinha.fedorapeople.org/axel/axel-2.4-1.fc10.src.rpm

all the other files from the mock build can be found at:

http://ankursinha.fedorapeople.org/axel/

regards,

Ankur Sinha

Hello, Ankur Sinha.

Is there any troubles with my package?

And why you use hardcoded path --prefix=/usr in their? Why not standard like %{_prefix}?


And, may be you wish review it now?

(In reply to comment #44)
> Hello, Ankur Sinha.
> 
> Is there any troubles with my package?

I haven't checked your package so I don't know.

> 
> And why you use hardcoded path --prefix=/usr in their? Why not standard like
> %{_prefix}?
> 
> 

Hmm.. ill check up.. thanks..

> And, may be you wish review it now?  

I can't. I'm a new packager, not a reviewer(at all!). I was packaging it for kicks and thought why not try co-maintaining it. :)

regards,

Ankur Sinha

Ok, I'm not argue - we can co-maintain it. When review will finished, I'll include you to owner in request.

(In reply to comment #45)
> I can't. I'm a new packager, not a reviewer(at all!). I was packaging it for
> kicks and thought why not try co-maintaining it. :)
BTW, you are packager, sponsored? Or you seek sponsor? Please keep in mind I can't sponsor you, I'm not in sponsor group.

(In reply to comment #46)
> Ok, I'm not argue - we can co-maintain it. When review will finished, I'll
> include you to owner in request.  

great :)

(In reply to comment #47)

> (In reply to comment #45)
> > I can't. I'm a new packager, not a reviewer(at all!). I was packaging it for
> > kicks and thought why not try co-maintaining it. :)
> BTW, you are packager, sponsored? Or you seek sponsor? Please keep in mind I
> can't sponsor you, I'm not in sponsor group.  

I'm a sponsored packager. I think the FE-NEEDSPONSOR blocker can be removed from the bug now??

and, made the change.. 

here are the new srpms and specs:
http://ankursinha.fedorapeople.org/axel/axel-2.4-1.fc10.src.rpm
http://ankursinha.fedorapeople.org/axel/axel-2.4-1.fc12.src.rpm
http://ankursinha.fedorapeople.org/axel/axel.spec

all other files are at : http://ankursinha.fedorapeople.org/axel/

regards,

Ankur

(In reply to comment #48)
> (In reply to comment #46)
> > Ok, I'm not argue - we can co-maintain it. When review will finished, I'll
> > include you to owner in request.  
> great :)
Sorry, it off course if you do not argue co-maintain it. If you wish, you can maintain axel single.

> I'm a sponsored packager. I think the FE-NEEDSPONSOR blocker can be removed
> from the bug now??
I think you must ask you sponsor do it.

Do you need now
for txt in CHANGES CREDITS API axelrc.example README ; do
	sed 's/\r//' $txt > $txt.new
	touch -r $txt $txt.new
	mv $txt.new $txt
done
?
As I see, this files uses UNIX-line endings now.

(In reply to comment #50)
> Do you need now
> for txt in CHANGES CREDITS API axelrc.example README ; do
>  sed 's/\r//' $txt > $txt.new
>  touch -r $txt $txt.new
>  mv $txt.new $txt
> done
> ?
> As I see, this files uses UNIX-line endings now.  

All this is trivial and will be corrected once the review process begins.. 

(In reply to comment #49)
> (In reply to comment #48)
> > (In reply to comment #46)
> > > Ok, I'm not argue - we can co-maintain it. When review will finished, I'll
> > > include you to owner in request.  
> > great :)
> Sorry, it off course if you do not argue co-maintain it. If you wish, you can
> maintain axel single.
> 
> > I'm a sponsored packager. I think the FE-NEEDSPONSOR blocker can be removed
> > from the bug now??
> I think you must ask you sponsor do it.  

The FE-NEEDSPONSOR is present since Ankur Srivastava at the time was not a
sponsored packager. Since we are both sponsored packagers, it is no longer
needed. 

Can I please take axel off your hands and maintain it alone (if you don't mind). I don't think the package needs 2 people maintaining it.. 

Anyway, Thanks a lot for your help :).

regards,

Ankur

hi,

I'm closing this bug report (due to inactivity of the reporter of the package) and starting a new review request here:

https://bugzilla.redhat.com/show_bug.cgi?id=510428

regards,

Ankur Sinha

*** This bug has been marked as a duplicate of bug 510428 ***