[Unbound-users] Segfault on user not found with 1.5.3

[Maciej Soltysiak]

Hi,

I just took the latest 1.5.3, compiled it and ran the executable without
doing any config. Just to see what happens.

It segfaulted after saying user unbound not found.

The issue is in util/config_file.c in function config_lookup_uid().

getpwnam() can return NULL and so subsequent referrences to pw_uid and
pw_gid are invalid.

This patch corrects the code path to assign uid and gid only if getpwnam()
is successful.

It also removes the log_err() call, because perform_setup() in
daemon/unbound.c would print it anyway in fatal_exit()

I know it's a tiny buglet, but I couldn't resist fixing a segfault :-)

Best regards,
Maciej Soltysiak

DNSCrypt Poland
https://dnscrypt.pl/

[W.C.A. Wijngaards]

Hi Maciej,

Post by Maciej Soltysiak
Hi,
I just took the latest 1.5.3, compiled it and ran the executable
without doing any config. Just to see what happens.
It segfaulted after saying user unbound not found.
The issue is in util/config_file.c in function
config_lookup_uid().
getpwnam() can return NULL and so subsequent referrences to pw_uid
and pw_gid are invalid.
This patch corrects the code path to assign uid and gid only if
getpwnam() is successful.
It also removes the log_err() call, because perform_setup() in
daemon/unbound.c would print it anyway in fatal_exit()
I know it's a tiny buglet, but I couldn't resist fixing a segfault :-)

Thank you for the report, applied it by setting checks on the success
of that lookup and continuing with the error printed.

Best regards,
Wouter

[Maciej Soltysiak]

Hi Wouter,

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi Maciej,

Post by Maciej Soltysiak
Hi,
I just took the latest 1.5.3, compiled it and ran the executable
without doing any config. Just to see what happens.
It segfaulted after saying user unbound not found.
The issue is in util/config_file.c in function
config_lookup_uid().
getpwnam() can return NULL and so subsequent referrences to pw_uid
and pw_gid are invalid.
This patch corrects the code path to assign uid and gid only if
getpwnam() is successful.
It also removes the log_err() call, because perform_setup() in
daemon/unbound.c would print it anyway in fatal_exit()
I know it's a tiny buglet, but I couldn't resist fixing a segfault :-)

Thank you for the report, applied it by setting checks on the success
of that lookup and continuing with the error printed.
Thanks for reviewing and applying.

What you applied to SVN is exactly what I've done initially.
That however made it first display "user 'unbound' does not exist." twice.
First from util/config_file.c. That allows the code run further, until it
encounters a getpwnam() call in perform_setup() of unbound.c where it has
fatal_exit() with the same message.

Confirmed with gdb.

That's why I removed the duplication to allow the latter error message to
show up.

I think you might want to look at it again; or maybe point me where I'm
making a mistake.

Best regards,
Wouter

Thanks,
Maciej

[W.C.A. Wijngaards]

Hi Maciej,

Post by Maciej Soltysiak
Hi Wouter,
On Mon, Mar 23, 2015 at 9:21 PM, W.C.A. Wijngaards
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Hi Maciej,

Post by Maciej Soltysiak
Hi,
I just took the latest 1.5.3, compiled it and ran the executable
without doing any config. Just to see what happens.
It segfaulted after saying user unbound not found.
The issue is in util/config_file.c in function
config_lookup_uid().
getpwnam() can return NULL and so subsequent referrences to
pw_uid and pw_gid are invalid.
This patch corrects the code path to assign uid and gid only if
getpwnam() is successful.
It also removes the log_err() call, because perform_setup() in
daemon/unbound.c would print it anyway in fatal_exit()
I know it's a tiny buglet, but I couldn't resist fixing a
segfault :-)

Thank you for the report, applied it by setting checks on the
success of that lookup and continuing with the error printed.
Thanks for reviewing and applying.
What you applied to SVN is exactly what I've done initially. That
however made it first display "user 'unbound' does not exist."
twice. First from util/config_file.c. That allows the code run
further, until it encounters a getpwnam() call in perform_setup()
of unbound.c where it has fatal_exit() with the same message.
Confirmed with gdb.
That's why I removed the duplication to allow the latter error
message to show up.
I think you might want to look at it again; or maybe point me where
I'm making a mistake.

Yes you are right, and I re-fixed it like your commit,

Best regards,
Wouter

[Maciej Soltysiak]

Post by W.C.A. Wijngaards
Yes you are right, and I re-fixed it like your commit,

Thanks Wouter,
Maciej