1) separate HTTP handling from SOAP handling
2) remove repeated validity check, each check is performed exactly once
3) fix HTTP status code per UPnP spec, SOAP spec and RFC 2774
Conflicts:
ChangeLog
upnp/src/soap/soap_device.c
Signed-off-by: Marcelo Roberto Jimenez <mroberto@users.sourceforge.net>
SF Bug Tracker #119, Creator: Klaus Fischer
Dear libupnp developers,
I have experienced a crash (access violation) when using libupnp on
Windows. The crash is actually located in pthreads-win32 and happens
when repeatedly de-/initializing libupnp on Win32 in the same process
and both libupnp and pthreads-win32 are compiled as static libraries.
So I'm doing this:
- UpnpInit()
- UpnpFinish()
- UpnpInit() <- Crash
I am already in touch with Ross Johnson on the pthreads-win32 mailing
list regarding this issue:
http://sourceware.org/ml/pthreads-win32/2013/msg00020.html
He told me the problem is that the functions
pthread_win32_process_attach/detach_np() should no longer be called
directly, but are invoked automatically now since version 2.9.0 of
pthreads-win32, which has been released approx. 1.5 years ago. Please
refer to above link for in-depth information.
So for proper using of latest pthreads-win32 library, those function
calls should vanish inside libupnp. Could you consider adapting libupnp
in that way? I would really like to use both libraries out-of-the-box
without local modifications, and this issue prevents that.
Best regards,
Klaus
(cherry picked from commit 2211cc14e644db48d8ea8dac34b02d6dc7b8c3c7)
SF Bug Tracker #118, Creator: T.Iwamoto
From: gon3456@users.sf.net
Steps to reproduce:
1. Extracts and build libupnp-1.6.18
$ tar -xjf /path/to/archive/libupnp-1.6.18.tar.bz2
$ cd libupnp-1.6.18
$ ./configure
$ make
2. Applies the attached patch and remake.
$ patch -p1 < /path/to/patch/libupnp-1.6.18.patch
$ make
3. Run tv_device.
$ cd upnp/sample
$ ./tv_device
4. Run tv_ctrlpt; the tv_ctrlpt crashes soon.
$ ./tv_ctrlpt
Segmentation fault (core dumped)
This is an issue report about the sample program of control point.
The tv_ctrlpt crashes after detecting a tvdevice that contains tvcontrol:2 or higher version of tvcontrol service.
tv_ctrlpt should detect correctly such devices due to forward compatibility of control points with device.
For more information about the compatibility, please refer the following document:
DLNA Architectures and Protocols Part 1 2011 December - 7.3.2.1.3 (GUN:GZJXU)
The attached patch changes the sample programs as below:
- device: changes version of tvcontrol service from 1 to 2. This change may occur in the future.
- cp: nothing changed: cp knows version 1 of tvcontrol service only.
I know many vendors implements their control points based on the tv_ctrlpt, so I hope to fix this issue ASAP.
==
From: Yoichi NAKAYAMA
SEGV is caused by strcpy with NULL argument.
Attached patch will avoid SEGV in strcpy, but there may be other inconsistencies.
> I know many vendors implements their control points based on the tv_ctrlpt,
I don't think so. I think tv_ctrlpt is just a sample to be used with tv_device.
Signed-off-by: Marcelo Roberto Jimenez <mroberto@users.sourceforge.net>
(cherry picked from commit 1a28e8ff517f58a174a26cbd15e1e9dd7d060127)
1) Only HTTP_XXX should be return
2) Make default return value work for process_request
(cherry picked from commit 06aa3b17c4e20c482de3865b4e8608077db8284b)
Conflicts:
upnp/src/genlib/net/http/webserver.c
Hi,
I had found some bugs about memory leak on libupnp-1.6.18.
It may lead to memory leak when calling ThreadPoolAdd() or
ThreadPoolAddPersistent() which does not return 0.
See the attachment for patch.
(cherry picked from commit 8e3a71905bc763332da9531285c507c2470989ae)
Conflicts:
upnp/src/ssdp/ssdp_ctrlpt.c
Suppose the UPnP device is listening on 192.168.1.102:49152. Use the following to send
garbage bytes to the device:
while true; do echo "\""; done | netcat 192.168.1.102 49152
The device just keeps receiving these bytes and its memory usage keeps growing.
Malicious client may exploit it to exhaust the device's memory.
The attached patch eliminates this possibility.
(cherry picked from commit 9a9c4e829ea059bafc80d5fb568223a8b2068b28)
1) restore the scanner's original cursor position in case of
insufficient input;
2) free the memories allocated for a new header in case of a failure.
(cherry picked from commit 7a571f513e801f071873f2627400461b04213b13)
it seems to me that there is still something wrong:
1) the new is_qdtext_char() is incorrect.
There is a trap if char is implemented as signed char.
Suppose that c is '\xFF', it will be -1 when converted to an int.
By definition, c should be qdtext:
qdtext = <any TEXT except <">>
TEXT = <any OCTET except CTLs, but including LWS>
OCTET = <any 8-bit sequence of data>
2) the character after '\\' could be either part of a quoted-pair
(together with '\\'), or a normal qdtext, since '\\' itself can
be treated as a qdtext. This is equivalent to saying that the
character after '\\' in a quoted string could be ANY octet.
A patch based on the above two observations is attached.
Peng
(cherry picked from commit f10730f616bc1ceff05b3987ffc8fcf3492f0f5b)
In soap_ctrlpt.c, in function get_response_value:
upnp_error_code is checked to see if it is less than 400 because that
would indicate a SOAP error code.
However it should be checked to see if it is greater than 400.
(cherry picked from commit 7b1aa4c9e30e95461dfc16fa8c30775fc2bba7cf)
What if pVirtualDirList has two nodes and what we want to delete is the
first one. Patch attached.
(cherry picked from commit 907c7c2621f8b2323b6bd8227ec082ce41c0a292)
Dear libupnp-devels,
when POST'ing to the simple web server in libupnp, the application crashes.
This is caused by a missing "..." argument in webserver.c:1533.
Seems it has been there for a long time ... 1.6.9 and 1.6.18 have it.
webserver.c:1533 calls http_MakeMessage
/* Send response. */
http_MakeMessage(&headers, 1, 1,
"RTLSXcCc",
ret, "text/html", X_USER_AGENT);
The format parameter RTLSXcCc needs four arguments -
R - response code - ret,
T- content type - text/html,
L - struct SendInstruction * - NOT PRESENT
X - user agent - X_USER_AGENT
This results in a crash.
Changing to
http_MakeMessage(&headers, 1, 1,
"RTLSXcCc",
ret, "text/html", &RespInstr, X_USER_AGENT);
solves the situation.
Yours,
Sebastian Brandt
(cherry picked from commit 25c27b8af7918a46c9a666e51a125e8c12c709d7)
This patch addresses three possible buffer overflows in function
unique_service_name(). The three issues have the folowing CVE
numbers:
CVE-2012-5958 Issue #2: Stack buffer overflow of Tempbuf
CVE-2012-5959 Issue #4: Stack buffer overflow of Event->UDN
CVE-2012-5960 Issue #8: Stack buffer overflow of Event->UDN
Notice that the following issues have already been dealt by previous
work:
CVE-2012-5961 Issue #1: Stack buffer overflow of Evt->UDN
CVE-2012-5962 Issue #3: Stack buffer overflow of Evt->DeviceType
CVE-2012-5963 Issue #5: Stack buffer overflow of Event->UDN
CVE-2012-5964 Issue #6: Stack buffer overflow of Event->DeviceType
CVE-2012-5965 Issue #7: Stack buffer overflow of Event->DeviceType
(cherry picked from commit f015a132e82239cbf40dcb2346d7bee83685531b)
Free buffers after malloc or ixmlPrintNode failure.
Free Param->Header before destructing Param.
(cherry picked from commit d9e90499b7ead7e9fae30b9c248b343ec6d9b273)
This patch fixes a bug in non blocking connect call where the sock
option length for SO_ERROR was passed as 0 instead of sizeof(int).
(cherry picked from commit 6db2271ac36adb6f5bc210240ed41ec8aad2c57f)
If device is disabled, SetGenaCallback definition is disabled,
but its call remains. A link error will occur in Win32.
(cherry picked from commit 4d21d45da730550707db5cdd2bca399f6217237f)
At the end of get_content_type() in webserver.c, it should check
return value of ixmlCloneDOMString().
(Forward ported from commit 708ea308060e3a68b9596a714afa94c56e6bbde4)
Wrong assignment by shutdown result hides the real error code
of NewRequestHandler() in ssdp_device.c.
Fix return code description of NewRequestHandler().
Handle return code from ithread_create in sample applications.
Remove unused assignments.
(forward ported from commit 39dfad2cba5f577a24659deb8afc81d9016b2080)
The problem can occur if one of the arguments is NULL.
Test argument and fix assertion.
(cherry picked from commit 3d7c1dbd5901300d0ad7db6653e8008aa54ab89c)
Sample applications share sample_util.c and collisions of
object file can occur in parallel build. Modify project files to
split intermediate directories against it.
Apply similar changes also to library projects, like vc10 projects.
(cherry picked from commit e52e7e0bd3e43df357e1586427e2264538013fd0)
In parser_parse_chunky_headers, parser->msg.msg.buf can be changed
by membuffer_delete call. Therefore if we save the pointer to
parser->msg.entity.buf before calling membuffer_delete, it will
induce access to released memory.
(cherry picked from commit d72bb5cff51bfc261de7725536cef53fd1f0b356)
1. Test Instr before dereference it in http_RecvPostMessage.
(Though it never becomes NULL because NULL is not passed to
the static method)
2. Avoid strdup(NULL) in ixmlElement_setAttributeNS.
Those are detected by llvm scan-build.
(cherry picked from commit a383cbb8e20545222880f064a5b27c15abbf9280)
Define http_gmtime_r and web_server_asctime_r and use it.
Those prefix are added since pthread for Win32 already
has macro gmtime_r and asctime_r.
(forward port of commit 34a77cc095a6be89a7cb2d71202364c3cc7e8d26)
The variable is declared as SOCKET, but it is used to
store return value of int receive_from_stopSock(...).
The type was changed in the commit
4b47e6a51d9c7049a862695b68de75699e023551 by mistake.
(cherry picked from commit e10bc2ec0ce5d84ef1adf1c90ae80810aab48afe)
Do not compile the code related to the webserver in http_SendMessage
when --disable-webserver is set
(cherry picked from commit 777c936f4cf022dd025f14bebc98a994a1498536)
Put thread in a detached state when calling pthread_create otherwise in
some circumstances, thread can end before the call to pthread_detach.
(cherry picked from commit dc4eda529f1da881cb986b95c6352583f120dd39)
Add --enable-unspecified_server configure option to set to "Unspecified"
the OS name, OS version, product name and product version normally
contained in the SERVER header as this could be used by an attacker.
(forward port of commit 49af91fe4892e30316cceb12eb50c3f2cafb73b4)
Submitted: Fabrice Fontaine ( ffontaine ) - 2012-03-29 07:36:34 PDT
Miniserver is disabled if ECXLUDE_GENA, EXCLUDE_SOAP and
EXCLUDE_WEBSERVER are set.
However, SSDP needs the Miniserver to answer to M-SEARCH requests.
So, MiniServer should not be disabled if EXCLUDE_SSDP is not also set.
(cherry picked from commit 9a33782ab54a456611c40251d366b3b58a70f6b5)
Use INCLUDE_DEVICE_APIS instead of UPNP_HAVE_DEVICE as in other sources.
Don't use soap_device_callback if INCLUDE_DEVICE_APIS is not set,
otherwise link error occur on Windows.
(cherry picked from commit e0e81e6cd2b90b5eda1f806540ceb17595ae149e)
Submitted: Yoichi NAKAYAMA ( yoichi ) - 2012-03-25 18:14:34 PDT
There are typos in upnp/src/inc/config.h "EXCLUDE_SSSDP" (shold be
EXCLUDE_SSDP), therefore EXCLUDE_SSDP is always 0, and --disable-ssdp
has no effect.
(cherry picked from commit 02afbb09c9dc0bde876f99a3a96f50ed655fd940)
Cast parameters of htonl in uint32_t in IN6_IS_ADDR_GLOBAL and
IN6_IS_ADDR_ULA definitions.
Remove comparison with 0 in while statement of vfmatch,
http_SendMessage and http_MakeMessage.
(cherry picked from commit 804088d85902305e949d9f7002d06b1b97a360da)
GetDeviceHandleInfo just fail without using undefined member DeviceAf
if UPNP_HAVE_DEVICE is not defined.
Move ContentTypeHeader definition to soap_common.c, since it is
also used in soap_ctrlpt.c.
(cherry picked from commit 679ebeec48d6fd64409bccea1cea466eee8dd237)
Submitted: Marco Virgulti ( mvirg83 ) - 2012-03-23 10:08:08 PDT
There is a problem, perhaps, during downloading a document by
UpnpDownloadXmlDoc. During debugging i've found that in an not exported
api (unfortunately i forgot the code line...) where it is setted a
local variable "int timeout" to -1 then passed directly to another
function for sending data through tcp socket. I patched this setting it
to 0 (there is an IF section that exits if timeout < 0). It is normal
behavior or it is a bug?
Add infoSize parameter to get_sdk_info function to replace sprintf call
by a snprintf call.
(cherry picked from commit e13ffe3bf84f7238b57bde73a4780cbf70bcc511)
