! Please note that this is a snapshot of our old Bugzilla server, which is read only since May 29, 2020. Please go to gitlab.xfce.org for our new server !
Incorrect getaddrinfo(3) usage in mailwatch plugin (patch included)
Status:
RESOLVED: FIXED
Product:
Xfce4-mailwatch-plugin
Component:
General

Comments

Description Adam Hoka 2007-12-27 18:47:06 CET
User-Agent:       Opera/9.24 (X11; Linux i686; U; en)
Build Identifier: 

In xfce-mailwatch-plugins 1.0.1, in the gmail, imap and pop3 code there is an incorrect usage of getaddrinfo(3), which results in a "EAI_BADHINTS, invalid value for hints" error message by NetBSD libc (glibc doesnt catch this apparently).

From NetBSD Library Functions Manual / getaddrinfo(3):

   (After ai_flags, ai_family, ai_socktype, ai_protocol)

     All other elements of the addrinfo structure passed via hints must be
     zero or the null pointer.

The Linux programmers manual also confirms it.

So, ai_addrlen should be zero, but currently it's "sizeof(struct sockaddr_in)".
It works correctly after applying the patches included below.

Here's the fix:
===============

--- panel-plugin/mailwatch-mailbox-pop3.c.orig  2007-12-26 19:00:36.000000000 +0000
+++ panel-plugin/mailwatch-mailbox-pop3.c
@@ -177,7 +177,7 @@ pop3_get_sockaddr(XfceMailwatchPOP3Mailb
                   const gchar *service, struct sockaddr_in *addr)
 {
     struct addrinfo hints = { 0, PF_INET, SOCK_STREAM, IPPROTO_TCP,
-            sizeof(struct sockaddr_in), NULL, NULL, NULL };
+            0, NULL, NULL, NULL };
     GError *error = NULL;

     TRACE("entering (%s, %s, %p)", host, service, addr);

--- panel-plugin/mailwatch-mailbox-imap.c.orig  2007-12-26 19:00:29.000000000 +0000
+++ panel-plugin/mailwatch-mailbox-imap.c
@@ -209,7 +209,7 @@ imap_get_sockaddr(XfceMailwatchIMAPMailb
                   const gchar *service, struct sockaddr_in *addr)
 {
     struct addrinfo hints = { 0, PF_INET, SOCK_STREAM, IPPROTO_TCP,
-            sizeof(struct sockaddr_in), NULL, NULL, NULL };
+            0, NULL, NULL, NULL };
     GError *error = NULL;

     TRACE("entering (%s, %s, %p)", host, service, addr);

--- panel-plugin/mailwatch-mailbox-gmail.c.orig 2007-12-26 19:00:22.000000000 +0000
+++ panel-plugin/mailwatch-mailbox-gmail.c
@@ -160,7 +160,7 @@ gmail_get_sockaddr(XfceMailwatchGMailMai
                   const gchar *service, struct sockaddr_in *addr)
 {
     struct addrinfo hints = { 0, PF_INET, SOCK_STREAM, IPPROTO_TCP,
-            sizeof(struct sockaddr_in), NULL, NULL, NULL };
+            0, NULL, NULL, NULL };
     GError *error = NULL;

     TRACE("entering (%s, %s, %p)", host, service, addr);


Reproducible: Always

Steps to Reproduce:
1. Compile it on NetBSD.
2. Set an imap, pop3 or gmail account.
3. Watch it failing.




NetBSD grimnismal 4.99.42 NetBSD 4.99.42 (REPLACED.NOACPI) #0: Mon Dec 24 23:31:12 UTC 2007  replaced@grimnismal:/var/tnf/obj/sys/arch/i386/compile/REPLACED.NOACPI i386
Comment 1 Brian J. Tarricone (not reading bugmail) 2008-01-31 00:59:52 CET
Certainly not critical.  Critical usually implies data loss.  Please don't mess with severity/prio.  That's generally for me to decide.
Comment 2 Adam Hoka 2008-01-31 01:28:43 CET
Sorry, I though you went missing or something, because I havent got any feedback.

BTW, any comment on the bugreport?
Comment 3 Brian J. Tarricone (not reading bugmail) 2008-01-31 01:34:31 CET
In principle I'm ok with the change you suggest, though I haven't had time to look into it recently.  A patch would certainly speed things up ^_~.
Comment 4 Adam Hoka 2008-01-31 01:50:46 CET
Created attachment 1504 
Patch 1
Comment 5 Adam Hoka 2008-01-31 01:51:00 CET
Created attachment 1505 
Patch 2
Comment 6 Adam Hoka 2008-01-31 01:51:18 CET
Created attachment 1506 
Patch 3
Comment 7 Adam Hoka 2008-01-31 01:54:31 CET
You obviously havent read the full bugreport, because the necessary changes were already there. ^_~
Comment 8 Brian J. Tarricone (not reading bugmail) 2008-01-31 02:00:02 CET
No, my attention is just divided, as I'm at work and am trying to get actual work done.   Just goes to show you that pasting patches inline isn't the best idea (also note that part of your inline patch has word-wrapped [at least for me], which will cause 'patch' to fail to apply it).  I did check the attachments area before asking for a patch, and didn't see one.

(For future reference: no need to separate patches per-file.  Perfectly fine to concatenate them into one file.  'patch' doesn't care.)
Comment 9 Adam Hoka 2008-01-31 02:30:01 CET
I just extracted the patches from the package for NetBSD as they were there.
But oh well, at least you have them now. 

I have read both of the NetBSD and Linux manpages and they both say it should be zero (netbsd libc also reports this as an error, glibc is possibly buggy for not detecting it, maybe I should also report it there). You can trust me on this one, but feel free to do some research if you want. I hope this bug has no security impact (I'm not a netcode/security expert so I cant tell), but consider releasing an errata.


I wish luck with your work,

Adam

PS.: BTW, nice plugin you wrote.
Comment 10 Brian J. Tarricone (not reading bugmail) 2008-01-31 11:40:43 CET
Ok, this should be fixed now.  I ended up not using your patches after all.  Turns out my use of getaddrinfo() was just totally incorrect, not just that problem.  So now it should be correct, and now it supports IPv6 as well.

Bug #3767

Reported by:
Adam Hoka
Reported on: 2007-12-27
Last modified on: 2011-02-26

People

Assignee:
Brian J. Tarricone (not reading bugmail)
CC List:
1 user

Version

Version:
1.1.0 or older
Target Milestone:
1.1.0 or older

Attachments

Patch 1 (525 bytes, patch)
2008-01-31 01:50 CET , Adam Hoka
no flags
Patch 2 (527 bytes, patch)
2008-01-31 01:51 CET , Adam Hoka
no flags
Patch 3 (523 bytes, patch)
2008-01-31 01:51 CET , Adam Hoka
no flags

Additional information