AbhishekGhosh / shellinabox

Automatically exported from code.google.com/p/shellinabox
Other
0 stars 0 forks source link

Segmentation fault solution #152

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
A lot of people are reporting segmentation fault in your shellinabox project.  
I experienced the same issue when I downloaded and built both 2.10 and the SVN 
tree.  I ran gdb to look at the issue, and found that it’s a null pointer 
deference in the he->h_name usage in libhttp/ssl.c line 934:

sslGenerateCertificate(defaultCertificate, he->h_name);

In this section of code:

      char hostname[256], buf[4096];
      check(!gethostname(hostname, sizeof(hostname)));
      struct hostent he_buf, *he;
      int h_err;
      if (gethostbyname_r(hostname, &he_buf, buf, sizeof(buf),
                          &he, &h_err)) {
        sslGenerateCertificate(defaultCertificate, hostname);
      } else {
        sslGenerateCertificate(defaultCertificate, he->h_name);
      }

Accessing *he in the case that gethostbyname_r returns null is going to cause 
undefined behavior, in this instance a null pointer dereference.  I think the 
if is simply upside down, which would use he->h_name if gethostbyname_r is 
successful, and use hostname if not.

Original issue reported on code.google.com by jerem...@freedomvoice.com on 23 Nov 2011 at 9:06

GoogleCodeExporter commented 9 years ago
Correction, line 676

Original comment by jerem...@freedomvoice.com on 23 Nov 2011 at 9:10

GoogleCodeExporter commented 9 years ago
Actually, gethostbyname_r returns an integer, which is 0 on success and nonzero 
on failure.  However, perhaps there are some circumstances under which 
gethostbyname_r fills he with a null pointer and returns 0 anyway, leaving an 
error code in h_err.

Original comment by andersk@mit.edu on 3 Jan 2012 at 11:20

GoogleCodeExporter commented 9 years ago
Either way, it wouldn't be a bad idea to null check he and use hostname if it's 
null after the call to gethostbyname_r

Original comment by bashar...@gmail.com on 4 Jan 2012 at 1:24

GoogleCodeExporter commented 9 years ago
Apparently it's best to use getaddrinfo() anyway, since gethostbyname_r is 
deprecated

Original comment by bashar...@gmail.com on 4 Jan 2012 at 1:58

GoogleCodeExporter commented 9 years ago
http://refspecs.linuxfoundation.org/LSB_3.2.0/LSB-Core-generic/LSB-Core-generic/
baselib-gethostbyname-r-3.html

"If no matching entry is found, *result shall be set to a null pointer."

So if gethostbyname_r is successful but no address matches, it returns zero and 
sets he to a null pointer.

Original comment by bashar...@gmail.com on 4 Jan 2012 at 2:04

GoogleCodeExporter commented 9 years ago
I am not personally experiencing this problem, but is this the fix you guys had 
in mind?

https://github.com/jayschwa/shellinabox/commit/06513c67126547add8d2cf24681ca02b0
9f26be3

Original comment by Jayschwa on 4 Jan 2012 at 9:37

GoogleCodeExporter commented 9 years ago
Jay's patch applied to 2.11.

Original comment by beewoo...@gmail.com on 31 Mar 2012 at 10:57