68 lines
2.4 KiB
Diff
68 lines
2.4 KiB
Diff
# UNDF: UNDF-2026-000000204
|
||
From 91fd961 Mon Sep 17 00:00:00 2001
|
||
Subject: [CWE-407] ssl_ncp: fix O(n²) cipher negotiation in ncp_get_best_cipher
|
||
|
||
ncp_get_best_cipher() iterated the server cipher list (outer strsep
|
||
loop) and for each token called tls_item_in_cipher_list(), which
|
||
allocates a copy of the peer list, walks it with strtok, and frees
|
||
it — O(m) work plus a malloc+free per outer iteration.
|
||
|
||
Total cost: O(n*m) comparisons + O(n) heap allocations per TLS
|
||
handshake, where n = len(server_list), m = len(peer_ncp_list).
|
||
|
||
Same root cause affects p2p_ncp_get_common_cipher() (ssl_ncp.c:388)
|
||
and dco_check_option_conflict() (dco.c:468).
|
||
|
||
Fix for ncp_get_best_cipher: split peer_ncp_list once into a small
|
||
stack-allocated array before the outer loop. Inner membership test
|
||
becomes a straight array scan — still O(m) but with no heap
|
||
allocation and cache-hot data. With typical lists of 3–8 ciphers
|
||
this is effectively O(1).
|
||
|
||
The same pattern should be applied to the other two call sites.
|
||
|
||
--- a/src/openvpn/ssl_ncp.c
|
||
+++ b/src/openvpn/ssl_ncp.c
|
||
@@ -246,6 +246,10 @@ ncp_get_best_cipher(const char *server_list, const char *peer_info,
|
||
const char *remote_cipher, struct gc_arena *gc)
|
||
{
|
||
+#define NCP_MAX_CIPHERS 32
|
||
+ const char *peer_arr[NCP_MAX_CIPHERS];
|
||
+ int peer_count = 0;
|
||
+
|
||
struct gc_arena gc_tmp = gc_new();
|
||
|
||
const char *peer_ncp_list = tls_peer_ncp_list(peer_info, &gc_tmp);
|
||
@@ -255,13 +259,31 @@ ncp_get_best_cipher(const char *server_list, const char *peer_info,
|
||
remote_cipher = "";
|
||
}
|
||
|
||
+ /* Split peer_ncp_list once into a fixed array — O(m) one-time cost,
|
||
+ * avoids O(n) repeated malloc+strtok inside the outer loop. */
|
||
+ {
|
||
+ char *tmp = string_alloc(peer_ncp_list, &gc_tmp);
|
||
+ char *tok = strtok(tmp, ":");
|
||
+ while (tok && peer_count < NCP_MAX_CIPHERS) {
|
||
+ peer_arr[peer_count++] = tok;
|
||
+ tok = strtok(NULL, ":");
|
||
+ }
|
||
+ }
|
||
+
|
||
char *tmp_ciphers = string_alloc(server_list, &gc_tmp);
|
||
|
||
const char *token;
|
||
while ((token = strsep(&tmp_ciphers, ":")))
|
||
{
|
||
- if (tls_item_in_cipher_list(token, peer_ncp_list) || streq(token, remote_cipher))
|
||
- {
|
||
+ int found = streq(token, remote_cipher);
|
||
+ if (!found) {
|
||
+ for (int pi = 0; pi < peer_count && !found; pi++)
|
||
+ found = (strcmp(token, peer_arr[pi]) == 0);
|
||
+ }
|
||
+ if (found)
|
||
break;
|
||
- }
|
||
}
|
||
|
||
char *ret = NULL;
|