fix(win32): DLL file/product version now set as per actual git tag version#3557
Open
VaibhavTekale1 wants to merge 275 commits intowarmcat:mainfrom
Open
fix(win32): DLL file/product version now set as per actual git tag version#3557VaibhavTekale1 wants to merge 275 commits intowarmcat:mainfrom
VaibhavTekale1 wants to merge 275 commits intowarmcat:mainfrom
Conversation
When the client goes away, on some platforms all we get is POLLIN revent stuck on... we have to read it to find out a zero length result and understand it's gone. Add SSE mode's DOING_TRANSACTION to the list of states we will read for.
This was originally commented because Coverity objected to it.
In fresh mbedtls rsa public decrypt is deprecated and not work on public keys. Replaced by lws_genrsa_hash_sig_verify.
In responses with channel numbers, the sender's channel number is sent instead of the recipient's channel number. See for example https://datatracker.ietf.org/doc/html/rfc4254#section-5.1 Putty uses channel 256 for session, and existing code break it with "Received SSH2_MSG_CHANNEL_OPEN_CONFIRMATION for nonexistent channel 0" error.
Added processing of window-change request, according to RFC 4254 6.7 https://datatracker.ietf.org/doc/html/rfc4254#section-6.7 For processing called callback pty_req with empty terminal and modes.
Fix minimal-mqtt-client and minimal-mqtt-client-multi from crashes by allowing indicating username and password are not on heap.
There's no problem for library build, also with LWS_WITH_MINIMAL_EXAMPLES, but after install at least on OSX, there are problems finding the installed lws include dir (concealed on most platforms by the path being in the default search list for the toolchain), and the references in the lws includes to the tls includes meaning that explicit paths for that must be available at consuming cmakes. This patch enhances the cmake config installed by lws to deal with adding the lws include paths to CMAKE_REQUIRED_INCLUDES and include_directories, so it can be found before the target is introduced. The tls include is passed back up the CMakeLists layers and the lws targets marked with target_include_directories(PUBLIC) with them, so they are understood as needed by consumers. More boilerplate is moved out of the example consuming cmakes. After this, on machines with previous installs of older lws, you may have to clean out the cmake install path, that is usually something like /usr/local/lib/cmake/libwebsockets/* before make installing lws and putting the latest content in there.
Either explicit option selection, or multiple SMP service threads, should enable SO_REUSEPORT warmcat#2470
It's not exported, it seems nothing wants it any more
mbedtls_x509_get_name() does not clean up properly after itself in the case of OOM on multi-segment name. We have to add extra handling and cleaning.
Coverity doesn't understand that since we already handled akid.keyIdentifier.MBEDTLS_PRIVATE(len) being zero, we don't need to check for akid.keyIdentifier.MBEDTLS_PRIVATE(p) being NULL. So explicitly check it, even though it is a NOP.
->san is a union, in this case we set the type to indicate we use SAN_OTHER_NAME member, but set the unstructured_name union member, that is smaller. This doesn't cause any problem, since the union has space for it. But Coverity noticed, it is wrong, so fix it.
Coverity gets antsy that we were checking wsi->a.protocol for NULL earlier then start using it... explicitly bail if it's NULL before we start using it.
continue here makes no difference than using break, but continue gets us a pointless complaint "statement continue does not have any effect" and break does not.
ip can't be NULL, it's set to the address of another object.
Coverity does not understand that once we checked that the header has a non-zero length, the associated pointer can never be NULL. Add a pointless check to make it happy.
warmcat#3329 Extra ~600 bytes stack needed might be a struggle for RTOS, trimmed from 10 recommended in issue.
…bled Signed-off-by: makejian <makejian@xiaomi.com>
Google's fuzzer platform blows a warning /src/libwebsockets/lib/plat/unix/unix-sockets.c:497:21: warning: implicit conversion loses integer precision: 'uint32_t' (aka 'unsigned int') to '__be16' (aka 'unsigned short') [-Wimplicit-int-conversion] 497 | sll.sll_protocol = (uint32_t)(htons((uint16_t)0x800));
/src/libwebsockets//lib/secure-streams/private-lib-secure-streams.h:214:1: warning: empty union has size 0 in C, size 1 in C++ [-Wextern-c-compat]
214 | union lws_ss_contemp {
A request like 'http://host?abc' set the QUERY_STRING variable for CGIs to "ab" instead of "abc". This was because the code didn't account for key-only fields, only for key-value ones.
The error path for the write failing closes the wsi, but returns 0, which indicates to the caller that the wsi is OK.
The handling of return codes in client-http generally can do with some improvement. Basically it should be OK (0) or the wsi should be closed synchronously in the function and return nonzero. Re-use LWS_HPI_RET_WSI_ALREADY_DIED as the closed return to make the meaning unambiguous. Add some extra use of bail3 exit path to cleanly return the closed value with cce for some extra cases.
In the case of "closed before connection", we need to use sizeof(_reason) - 1 warmcat#3380
We need to hold back a few bytes of our max usage of the cgi stdout in buffer to allow for chunking header size, otherwise we run the risk of buffer overrun trashing other stack vars. warmcat#3402
The files involved already have proper copyright notices. This adds them to the license summary in ./LICENSE.
We may paint vhosts with plugin protocols, but that doesn't mean that the protocol is enabled on the vhost. In turn that means if we didn't do PROTOCOL_INIT, we should not do PROTOCOL_DESTROY on the vhost-protocol combo.
For the edge case that a client wsi was created but did not get the chance to go around the event loop even once, introduce a pt dll2_owner that lists wsi that have been created in the heap, but not bound to any fds entry. __lws_wsi_create_with_role() adds the created wsi to the pt, and subsequently it's removed from the list when an fd is added to the fds. At pt destroy time, the listed wsi are walked and freed.
verify_options in mbedtls-server is slightly different from openssl-server. So made mbedtls-server have the same semantic about verify_option. And also warmcat#2902 was the workaround fix for the 'weird' verify_option, so changed auth_mode to match verify_option's meaning. this patch is basically warmcat#3257 for 4.3-stable
This document contains sensitive information collected during our security research activities related with the Libwebsockets library maintained by Andy Green (warmcat). +-------------------------------------------------------------------------------------------------------+ | Report information | +:===================================:+:===============================================================:+ | Vendor | warmcat | +-------------------------------------+-----------------------------------------------------------------+ | Vendor URL | https://libwebsockets.org/git/libwebsockets | +-------------------------------------+-----------------------------------------------------------------+ | Affected component | libwebsockets | +-------------------------------------+-----------------------------------------------------------------+ | Affected version | 4.4 | +-------------------------------------+-----------------------------------------------------------------+ | Vulnerability | CWE-416: Use After Free | +-------------------------------------+-----------------------------------------------------------------+ | Proposed CVSS v3.1 Base Score | 6.0 | +-------------------------------------+-----------------------------------------------------------------+ | Proposed CVSS v3.1 Vector | CVSS:4.0/AV:N/AC:L/AT:P/PR:N/UI:P/VC:N/VI:N/VA:H/SC:N/SI:N/SA:N | +-------------------------------------+-----------------------------------------------------------------+ +-----------------------------------------------------------------------------+ | Security Researcher(s) | +:===================================:+:=====================================:+ | Name | **Email address** | +-------------------------------------+---------------------------------------+ | Raffaele Bova | labs-advisory@nozominetworks.com | +-------------------------------------+---------------------------------------+ Libwebsockes is a C library that provides client and server implementation for various protocols (e.g., HTTP, websockets, MQTT) and more. Nozomi Networks Lab discovered a "CWE-416: Use After Free" in the latest software version of libwebsockets, specifically in the WebSocket server implementation. Depending on the use of the API, the vulnerability may allow an attacker to read or write data, that could cause a loss of integrity or availability. The issue is caused by the `lws_handshake_protocol` function, specifically when the upgrade header is not valid, the function calls `lws_http_transaction_completed`, which frees some of the data in the wsi structure, then it calls `user_callback_handle_rxflow` passing the up pointer and uses it on following strcasecmp calls. From our understanding, for this vulnerability to have a meaningful impact, a user that implements the Websocket server, must provide a user callback function which is going to handle `LWS_CALLBACK_HTTP_CONFIRM_UPGRADE`, while ignoring the length and doing operations on the up pointer. It is possible to compile the minimal websocket server using address sanitizer, to quickly verify the use after free. From our understanding of the code, if the upgrade header does not match the intended contents, then the code after the if statement when `lws_http_transaction_completed` is called, should not be executed, thus simply enclosing all that code in the else branch solves the issue.
This document contains sensitive information collected during our security research activities related with the Libwebsockets library made by Andy Green (warmcat). +-------------------------------------------------------------------------------------------------------+ | Report information | +:===================================:+:===============================================================:+ | Vendor | warmcat | +-------------------------------------+-----------------------------------------------------------------+ | Vendor URL | https://libwebsockets.org/git/libwebsockets | +-------------------------------------+-----------------------------------------------------------------+ | Affected component | Ecostruxure Automation Expert | +-------------------------------------+-----------------------------------------------------------------+ | Affected version | 4.4 | +-------------------------------------+-----------------------------------------------------------------+ | Vulnerability | CWE-121: Stack-based Buffer Overflow | +-------------------------------------+-----------------------------------------------------------------+ | Proposed CVSS v3.1 Base Score | 7.5 | +-------------------------------------+-----------------------------------------------------------------+ | Proposed CVSS v3.1 Vector | CVSS:4.0/AV:A/AC:L/AT:P/PR:N/UI:P/VC:H/VI:H/VA:H/SC:N/SI:N/SA:N | +-------------------------------------+-----------------------------------------------------------------+ +-----------------------------------------------------------------------------+ | Security Researcher(s) | +:===================================:+:=====================================:+ | Name | **Email address** | +-------------------------------------+---------------------------------------+ | Raffaele Bova | labs-advisory@nozominetworks.com | +-------------------------------------+---------------------------------------+ **\** Libwebsockes is a C library that provides client and server implementation for various protocols (e.g., HTTP, websockets, MQTT) and more. Nozomi Networks Lab discovered a "CWE-121: Stack-based Buffer Overflow" in the latest software version of libwebsockets, specifically in the async-dns component. The vulnerability allows an attacker that can inspect DNS requests made by the victim (e.g. being in the same wireless network) to forge a DNS response packet that overflows the stack and may lead to arbitrary code execution (depending on the platform and compiler options). The issue resides in `lws_adns_parse_label` function in `lib/system/async-dns/async-dns-parse.c`; this function iteratively parses a label however it does not correctly check the number of bytes written in the destination buffer. Specifically, the size of the dest output buffer is specified in the `dl` argument, however during the read of each substring of the label only the length of the current substring of the label is accounted for not overflowing the destination buffer, but previous reads are not accounted for. This means that a label of arbitrary size and content can be supplied and is copied onto the stack, however it must be split into substrings of size less than `dl`. To trigger the vulnerability an attacker must be able to sniff the DNS request packet to send a response with a matching identifier, otherwise the implantation correctly ignores the response. We have provided a harness for testing, for ease of use copy the harness in a subdirectory, for example in minimal-examples-lowlevel/api-tests/, and build it ``` cmake -B build -DLWS_WITH_SYS_ASYNC_DNS=1 -DLWS_WITH_SSL=0 -DCMAKE_C_FLAGS="-fsanitize=address" . && make -C build lws-test-async-dns ``` Then it can be run `./build/bin/lws-test-async-dns < poc_stackbof`  We suggest keeping track of the number of bytes currently written on the dest buffer, this could be done by saving the original dest pointer, decrementing dl on each substring memcpy, or using an auxiliary variable.
Member
|
Thanks... you're right it didn't take care about bumping the release version. However since then, there is a v4.3.10 tag, which does bump the release version (despite the patch doing that calling itself v4.3.8... it does set it to v4.3.10 though). Is that enough to solve this or does your change do something else? |
lws-team
force-pushed
the
main
branch
3 times, most recently
from
6790fe8 to
609de65
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@lws-team
Problem : The websockets.dll on Windows reported incorrect version information in its file properties.
File Version showed 4.3.0.0 — the patch component was always hardcoded to 0 because FILEVERSION and PRODUCTVERSION in version.rc.in were using the LWS_NUMVERSION macro, which was defined as MAJOR,MINOR,0 (patch intentionally omitted).
Product Version showed 4.3.5 — the patch number (5) was hardcoded in CMakeLists.txt:413 as CPACK_PACKAGE_VERSION_PATCH_NUMBER and was never updated to match the actual tagged release. So even when the repository was at tag v4.3.7, the DLL still reported 4.3.5.
This means the version embedded in the DLL was both structurally wrong (missing patch in FILEVERSION) and manually out-of-date (wrong patch value), making it unreliable for diagnostics and release verification.
With added fix :
Two changes were made:
After git describe --tags runs and stores its result in GIT_HASH,
a regex is applied to parse the semver components (vX.Y.Z) out of the tag. If a valid version tag is found, it overrides the hardcoded CPACK_PACKAGE_VERSION_MAJOR, CPACK_PACKAGE_VERSION_MINOR, and CPACK_PACKAGE_VERSION_PATCH_NUMBER.
The hardcoded values remain as a safe fallback when git is unavailable.
This ensures the version numbers used throughout the build always reflect the actual checked-out tag without requiring any manual update to CMakeLists.txt on each release.
Changed FILEVERSION and PRODUCTVERSION from using LWS_NUMVERSION (MAJOR,MINOR,0) to LWS_VERSION (MAJOR,MINOR,PATCH,0). This ensures the binary-encoded version fields in the PE resource — which tools like Windows Explorer, sigcheck, and installers read — carry the correct and complete version number.
Result after fix (for tag v4.3.7):
Before fix DLL properties :
After fix :
