RetroShare / RetroShare

RetroShare is a Free and Open Source cross-platform, Friend-2-Friend and secure decentralised communication platform.
https://retroshare.cc/
Other
1.75k stars 279 forks source link

RetroShare/libretroshare/src/util/dnsresolver.cc #125

Closed textbrowser closed 9 years ago

textbrowser commented 9 years ago

gethostbyname() may use static data. Please consider using the reentrant version gethostbyname_r() or protect the function with some locking mechanism.

There is a lock prior to if(!next_call.empty()), however, it's destroyed as soon as its parent begin-end block completes.

If you only have one DNSResolver object, you're probably fine. Although, if getPage() (see below) is executed in a separate thread, you may have unpleasant results.

libretroshare/src/util/extaddrfinder.cc also refers to gethostbyname().

Qt includes QHostInfo (http://doc.qt.io/qt-4.8/qhostinfo.html). It does not appear to support a means of specifying a DNS proxy.

csoler commented 9 years ago

Using Qt in libretroshare is not possible. We try to keep this part of the code Qt-free. You seem quite efficient in fixing bugs. If you want to send us pull requests, you're welcome!

textbrowser commented 9 years ago

Alright, will provide a possible solution using gethostbyname_r().

textbrowser commented 9 years ago

I have not tested the following.

...
if(!next_call.empty())
{
 char *buffer = (char *) malloc(8192);

 if(!buffer)
  return; // Do something.

 int error = 0;
 struct hostent pHost;
 struct hostent *result;

 if(gethostbyname_r(next_call.c_str(), &pHost, buffer, sizeof(buffer), &result, &error) == 0)
  if(result)
   // Use contents of result.
...
 free(buffer);

You may wish to consider an incremental lookup if you believe that the above buffer is too large. gethostbyname_r() will return ERANGE if the provided buffer is too small.

csoler commented 9 years ago

done. Thx!