Skip to content

Commit 6f3335c

Browse files
bugfix: three latent bugs found in src code review
- ssl_certby: avoid double-free of name_chain after SSL_set_client_CA_list transfers ownership; clear it so the failed: path does not free it again. - regex: PCRE1 ngx_stream_lua_regex_compile failed: path returned NGX_OK instead of NGX_ERROR, masking pcre_fullinfo() failure and leaving captures uninitialized (matches PCRE2 sibling now). - output: fix 4-byte buffer overflow when a non-NULL lightuserdata is passed; the size pass reserves nothing for it but the copy pass wrote "null". Guard the copy with the same NULL check in both ngx.echo and copy_str_in_table.
1 parent 421ed88 commit 6f3335c

3 files changed

Lines changed: 26 additions & 10 deletions

File tree

src/ngx_stream_lua_output.c

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -201,10 +201,16 @@ ngx_stream_lua_ngx_echo(lua_State *L, unsigned newline)
201201
break;
202202

203203
case LUA_TLIGHTUSERDATA:
204-
*b->last++ = 'n';
205-
*b->last++ = 'u';
206-
*b->last++ = 'l';
207-
*b->last++ = 'l';
204+
/* must mirror the size pass above: only NULL lightuserdata
205+
* (ngx.null) reserves space for "null"; non-NULL reserves
206+
* nothing, so write nothing here to avoid overflowing the
207+
* buffer. */
208+
if (lua_touserdata(L, i) == NULL) {
209+
*b->last++ = 'n';
210+
*b->last++ = 'u';
211+
*b->last++ = 'l';
212+
*b->last++ = 'l';
213+
}
208214
break;
209215

210216
default:
@@ -426,10 +432,15 @@ ngx_stream_lua_copy_str_in_table(lua_State *L, int index, u_char *dst)
426432

427433
case LUA_TLIGHTUSERDATA:
428434

429-
*dst++ = 'n';
430-
*dst++ = 'u';
431-
*dst++ = 'l';
432-
*dst++ = 'l';
435+
/* must mirror ngx_stream_lua_calc_strlen_in_table: only NULL
436+
* lightuserdata (ngx.null) reserves space for "null"; non-NULL
437+
* reserves nothing, so write nothing here. */
438+
if (lua_touserdata(L, -1) == NULL) {
439+
*dst++ = 'n';
440+
*dst++ = 'u';
441+
*dst++ = 'l';
442+
*dst++ = 'l';
443+
}
433444
break;
434445

435446
default:

src/ngx_stream_lua_regex.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ ngx_stream_lua_regex_compile(ngx_stream_lua_regex_compile_t *rc)
329329

330330
rc->err.len = ngx_snprintf(rc->err.data, rc->err.len, p, &rc->pattern, n)
331331
- rc->err.data;
332-
return NGX_OK;
332+
return NGX_ERROR;
333333
}
334334
#endif
335335

src/ngx_stream_lua_ssl_certby.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1666,8 +1666,13 @@ ngx_stream_lua_ffi_ssl_verify_client(ngx_stream_lua_request_t *r,
16661666
}
16671667
}
16681668

1669-
/* clean subject name list, and set it for send to client */
1669+
/* clean subject name list, and set it for send to client.
1670+
* ownership of name_chain is transferred to ssl_conn here, so
1671+
* clear our reference to avoid a double-free on the failed: path
1672+
* if a later step (trusted_chain loop or set0_verify_cert_store)
1673+
* fails. */
16701674
SSL_set_client_CA_list(ssl_conn, name_chain);
1675+
name_chain = NULL;
16711676
}
16721677

16731678
if (trusted_chain != NULL) {

0 commit comments

Comments
 (0)