# addressing all remaining code scanning instances of warning C6011,
null pointer dereference
this pr aims to address more static code analyser warnings, specifically
null pointer dereferences. the majority of changes are solely to quieten
the analyser, as `malloc` and `calloc` are unlikely to fail, but this
should at least lead to the code analysis being more readable and
usable.
where functions addressed had existing failure strategies, they were
used, however some functions will now silently fail rather than
attempting to dereference a null pointer. if there is a preferred
solution in these cases, i will be happy to implement it.
---
this is an extension of [this pull
request](https://github.com/coturn/coturn/pull/1729)
Marking variables as const when they won't be modified after
initialization helps programmers trying to understand a codebase to
manage the cognative load.
This pull request uses a clang-tidy fixit (Hard to automate, since the
code needs to be temporarily compiled as C++ for it to work) to try to
mechanically apply the const keyword to code where the automated tool
can determine that the variable won't be modified.
I then follow this up with a manual improvement pass to
turnutils_uclient, where I address const correctness of local variables,
as well as do some adjustments to loops and scoping to help with
reducing complexity.
Co-authored-by: redraincatching <redraincatching@disroot.org>
Co-authored-by: Pavel Punsky <eakraly@users.noreply.github.com>
This PR fixes an issue in the Makefile. Specifically, previously, any
modifications of files like src/apps/common/ns_turn_openssl.h would not
trigger a rebuild of build/obj/ns_turn_msg.o. The PR fixes this by
including them as additional dependencies.
allow fuzzing to be performed as part of the continuous integration.
the timing of the fuzzing can be extended, and i aim to broaden the
scope of the fuzz testing as well, since we currently only test the stun
message parser.
In decrypt_aes_128() at src/apps/replay/mainreplay.c, it calls
base64decode() to allocates memory in encryptedText, but forgets to free
encryptedText in the end of this function. Add free() after finished
using encryptedText.
Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn>
When using --log-file stdout to redirect turnserver logs to stdout
instead of creating a log file, a default log file was still being
created. This happened because the --log-file argument was processed
after logging had already occurred during startup.
Fix#1670
The `Source0` field in `rpm/turnserver.spec` was pointing to a dead link
at
`http://turnserver.open-sys.org/downloads/v%{version}/%{name}-%{version}.tar.gz`.
The domain `turnserver.open-sys.org` is no longer resolvable, making RPM
package building impossible when relying on this source.
**Changes made:**
- Updated `Source0` to use GitHub archive URL:
`https://github.com/coturn/coturn/archive/refs/tags/upstream/%{version}.tar.gz`
- Updated version from `4.7.0` to `4.5.2` to match the latest available
GitHub tag (`upstream/4.5.2`)
**Verification:**
- ✅ Old URL confirmed dead (domain resolution fails)
- ✅ New GitHub archive URL works correctly and downloads valid gzipped
tar archive
- ✅ RPM spec file syntax validation passes (`rpmbuild --nobuild`)
- ✅ URL properly expands with RPM variable substitution
The new source URL format follows GitHub's standard archive pattern and
will work reliably for future RPM builds.
Fixes#1574.
> [!WARNING]
>
> <details>
> <summary>Firewall rules blocked me from connecting to one or more
addresses</summary>
>
> #### I tried to connect to the following addresses, but was blocked by
firewall rules:
>
> - `turnserver.open-sys.org`
> - Triggering command: `curl -I REDACTED` (dns block)
>
> If you need me to access, download, or install something from one of
these locations, you can either:
>
> - Configure [Actions setup
steps](https://gh.io/copilot/actions-setup-steps) to set up my
environment, which run before the firewall is enabled
> - Add the appropriate URLs or hosts to my [firewall allow
list](https://gh.io/copilot/firewall-config)
>
> </details>
<!-- START COPILOT CODING AGENT TIPS -->
---
💬 Share your feedback on Copilot coding agent for the chance to win a
$200 gift card! Click
[here](https://survey.alchemer.com/s3/8343779/Copilot-Coding-agent) to
start the survey.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: ggarber <512252+ggarber@users.noreply.github.com>
implemented change suggested in TODO to speed up aes key generation
without, hopefully, negatively impacting the overall randomness of the
function
---------
Co-authored-by: Gustavo Garcia <gustavogb@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This PR fixes a null pointer dereference vulnerability where
`set_ssl_ctx()` could crash when passed a NULL engine handle.
## Problem
The `create_ioa_engine()` function can return NULL when invalid
parameters are provided:
```c
if (!relays_number || !relay_addrs || !tp) {
TURN_LOG_FUNC(TURN_LOG_LEVEL_ERROR, "%s: Cannot create TURN engine\n", __FUNCTION__);
return NULL;
}
```
However, two calling functions don't check for NULL before passing the
result to `set_ssl_ctx()`:
1. **`setup_relay_server()`** (line 1646):
```c
rs->ioa_eng = create_ioa_engine(...);
set_ssl_ctx(rs->ioa_eng, &turn_params); // Potential NULL dereference
```
2. **`create_new_listener_engine()`** (line 955):
```c
ioa_engine_handle e = create_ioa_engine(...);
set_ssl_ctx(e, &turn_params); // Potential NULL dereference
```
The `set_ssl_ctx()` function then dereferences the engine parameter
without checking:
```c
struct event_base *base = e->event_base; // Crashes if e is NULL
```
## Solution
Added a simple check before calling `set_ssl_ctx()`:
## Impact
- ✅ Prevents crashes when `create_ioa_engine()` fails due to invalid
configuration
- ✅ Minimal change with no functional impact on normal operation
- ✅ All existing tests continue to pass
- ✅ Follows defensive programming best practices
Fixes#1718.
This PR adds a new `--cpus` configuration option to address CPU
detection issues in virtualized and containerized environments where
`_SC_NPROCESSORS_CONF` and `_SC_NPROCESSORS_ONLN` return host CPU counts
instead of allocated container CPUs.
## Problem
In containerized deployments, coturn detects the host's CPU count (e.g.,
128 CPUs) instead of the container's allocated CPUs (e.g., 2 CPUs). This
causes the server to create excessive relay threads and database
connections, leading to resource exhaustion and performance issues.
## Solution
Added a new `cpus` configuration option that allows manual override of
CPU detection:
### Command Line Usage
```bash
turnserver --cpus 2
```
### Configuration File Usage
```ini
# Override system CPU count detection for containers
cpus=2
```
## Key Features
- **Backward Compatible**: No changes needed for existing deployments
- **Input Validation**: Values must be between 1 and 128 with proper
error handling
- **Comprehensive Documentation**: Updated man pages and example config
files
- **Both Interfaces**: Works via command line and configuration file
## Testing
The implementation has been thoroughly tested:
```bash
# Container with 2 allocated CPUs on 128-CPU host
$ turnserver --cpus 2
INFO: System cpu num is 128 # Host detection
INFO: System enable num is 128 # Host detection
INFO: Configured cpu num is 2 # Override applied
INFO: Total General servers: 2 # Correct thread count
```
- ✅ Command line option: `--cpus 8` creates 8 relay servers
- ✅ Config file option: `cpus=6` creates 6 relay servers
- ✅ Error handling: Invalid values show appropriate errors
- ✅ Default behavior: Without option, uses system detection
- ✅ RFC5769 tests: All protocol tests still pass
## Files Modified
- `src/apps/relay/mainrelay.c` - Core implementation
- `src/apps/relay/mainrelay.h` - Added configuration flag
- `examples/etc/turnserver.conf` - Added documentation and example
- `man/man1/turnserver.1` - Updated man page
This change directly addresses the resource consumption issues in
containerized environments while maintaining full backward
compatibility.
Fixes#1628.
### Describe
Hi,
Fixes resource and memory leaks in `udp_create_server_socket()` by
ensuring that the socket file descriptor (`udp_fd`) and dynamically
allocated memory (`server_addr`) are properly released on failure.
Specifically, if `addr_bind()`, `event_new()`, or `event_add()` fails,
the function now closes the socket and frees memory to prevent leaks.
### Expected Behavior
On any failure during socket binding or event registration, both
`udp_fd` and `server_addr` should be released to avoid leaking system
resources.
### Actual Behavior
Previously, if `addr_bind()`, `event_new()`, or `event_add()` failed,
the function would return early without closing the socket or freeing
memory, causing file descriptor and heap memory leaks.
This patch addresses overlooked memory and resource cleanup on failure
paths, improving server stability through targeted and essential
changes.
Thanks for reviewing.
Co-authored-by: Gustavo Garcia <gustavogb@gmail.com>
@ggarber i noticed too late that i used `0x03` instead of `0x02` by
mistake - this is an issue because it means that `add_requested_family`
will never be set when ipv6 is being used, so this should be fixed
removing an unnecessary null check as raised in [this code
scan](https://github.com/redraincatching/coturn/security/code-scanning/179)
the variable is confirmed to not be null in an outer loop guard
also changed the name of the variable in `rtcp_map_put`'s function
declaration to match that used in its function definition
Two compiler warnings were addressed:
* In `src/apps/relay/http_server.c`, line 77, a `-Wpointer-sign` warning
occurred when initializing a `char *` with the `uint8_t *` return type
of `ioa_network_buffer_data()`.
* An explicit cast `(char *)` was added to
`ioa_network_buffer_data(nbh_http)` to resolve the type mismatch.
* In `src/apps/relay/acme.c`, line 59, a `-Wchar-subscripts` warning was
present because a `char` variable `c` was used as an array index. `char`
can be signed, potentially leading to negative indices.
* Initially, `c` was cast to `(unsigned char)` at the point of use:
`A[(unsigned char)c]`.
* This was later improved by changing the declaration of `c` from `const
char` to `const unsigned char c = req[k]
---------
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Fix issues with Global Allocation Count for drain mode
- move increment/decrement logic out of userdb.c and tie to Prometheus
logic for allocation tracking instead
- log global allocation count decrements at INFO level, when drain mode
is on
Set new release version to 4.7.0
Updating minor version due to some breaking changes in options to enable
more secure/robust configuration without additional flags (or relying on
recommended conf file which people seem to skip during updates)
TLSv1 and TLSv1.1 can be enabled using `--tlsv1` and `--tlsv1_1`
arguments accordingly
That assumes openssl version being used has these versions enabled
(which as of openssl-3.5 is not by default)
With notable exceptions of:
src/apps/common/win/*
src/apps/relay/telnet.*
The purpose of this change is to add the SPDX tags from
https://spdx.dev/, which is a linux foundation project, to the source
code.
This provides automated code provenance tools, which are used in setting
up software bill of materials reports, an easy time verifying that the
code license is known and no incompatibilities are present in a
codebase.
No copyright date, author, or license changes are made.
Note also that
7e525c8e1c
is the original commit for the ACME code (acme.h and acme.c) which was
then moved to acme.h and acme.c in this commit
d4686750ee
but neither commit indicates what license the ACME code was submitted
as.
https://github.com/coturn/coturn?tab=License-1-ov-file#readme is the
3-clause BSD license, but https://github.com/coturn/coturn/pull/672
documents that the author's intent was for the MIT license. So I've used
the SPDX tag and content of the MIT license for this change.
Deprecate `--no-stun-backward-compatibility` and set it to true by
default
Add new option `--stun-backward-compatibility`, off by default
Update example/recommended configuration files
This is a breaking change as passing `--no-stun-backward-compatibility`
will be rejected as invalid argument
Invert `--no-rfc5780` option to be true by default
Make it `--rfc5780` to enable it
Update example/recommended configuration files
Passing `--no-rfc5780` will have no effect as this is the default
behavior now
Add basic diagram with modules of coturn inside Developer.md.
I also wanted to move the existing Flow chart to mermaid so that our doc
doesn't depend on external tools but i couldn't find the time to do it
properly (horizontal arrows are hard to represent with mermaid) so i
postponed that part.
This is the result of the mermaid diagram:
When building with default configuration at least in MacOS we get
warnings about those conversions from const char* to char*. Fix it by
making all those argv ""const char*"
Pavel Punsky
