Skip to content

Commit 38f587a

Browse files
doujiang24doujiang24
authored
bugfix: ensured arguments of APIs mutating uri or request/response headers do not contain unsafe characters. (#1654)
Signed-off-by: Thibault Charbonnier <[email protected]>
1 parent be864c2 commit 38f587a

13 files changed

+433
-80
lines changed

‎README.markdown

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4251,10 +4251,8 @@ to be returned when reading `ngx.header.Foo`.
42514251

42524252
Note that `ngx.header` is not a normal Lua table and as such, it is not possible to iterate through it using the Lua `ipairs` function.
42534253

4254-
Note: `HEADER` and `VALUE` will be truncated if they
4255-
contain the `\r` or `\n` characters. The truncated values
4256-
will contain all characters up to (and excluding) the first occurrence of
4257-
`\r` or `\n`.
4254+
Note: this function throws a Lua error if `HEADER` or
4255+
`VALUE` contain unsafe characters (control characters).
42584256

42594257
For reading *request* headers, use the [ngx.req.get_headers](#ngxreqget_headers) function instead.
42604258

@@ -4499,6 +4497,9 @@ which is functionally equivalent to
44994497
}
45004498
```
45014499

4500+
Note: this function throws a Lua error if the `uri` argument
4501+
contains unsafe characters (control characters).
4502+
45024503
Note that it is not possible to use this interface to rewrite URI arguments and that [ngx.req.set_uri_args](#ngxreqset_uri_args) should be used for this instead. For instance, Nginx config
45034504

45044505
```nginx
@@ -4914,6 +4915,9 @@ is equivalent to
49144915
ngx.req.clear_header("X-Foo")
49154916
```
49164917

4918+
Note: this function throws a Lua error if `header_name` or
4919+
`header_value` contain unsafe characters (control characters).
4920+
49174921
[Back to TOC](#nginx-api-for-lua)
49184922

49194923
ngx.req.clear_header
@@ -5247,12 +5251,8 @@ ngx.redirect
52475251

52485252
Issue an `HTTP 301` or `302` redirection to `uri`.
52495253

5250-
Notice: the `uri` should not contains `\r` or `\n`, otherwise, the characters after `\r` or `\n` will be truncated, including the `\r` or `\n` bytes themself.
5251-
5252-
The `uri` argument will be truncated if it contains the
5253-
`\r` or `\n` characters. The truncated value will contain
5254-
all characters up to (and excluding) the first occurrence of `\r` or
5255-
`\n`.
5254+
Note: this function throws a Lua error if the `uri` argument
5255+
contains unsafe characters (control characters).
52565256

52575257
The optional `status` parameter specifies the HTTP status code to be used. The following status codes are supported right now:
52585258

‎doc/HttpLuaModule.wiki

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3533,10 +3533,8 @@ to be returned when reading <code>ngx.header.Foo</code>.
35333533
35343534
Note that <code>ngx.header</code> is not a normal Lua table and as such, it is not possible to iterate through it using the Lua <code>ipairs</code> function.
35353535
3536-
Note: <code>HEADER</code> and <code>VALUE</code> will be truncated if they
3537-
contain the <code>\r</code> or <code>\n</code> characters. The truncated values
3538-
will contain all characters up to (and excluding) the first occurrence of
3539-
<code>\r</code> or <code>\n</code>.
3536+
Note: this function throws a Lua error if <code>HEADER</code> or
3537+
<code>VALUE</code> contain unsafe characters (control characters).
35403538
35413539
For reading ''request'' headers, use the [[#ngx.req.get_headers|ngx.req.get_headers]] function instead.
35423540
@@ -3746,6 +3744,9 @@ which is functionally equivalent to
37463744
}
37473745
</geshi>
37483746
3747+
Note: this function throws a Lua error if the <code>uri</code> argument
3748+
contains unsafe characters (control characters).
3749+
37493750
Note that it is not possible to use this interface to rewrite URI arguments and that [[#ngx.req.set_uri_args|ngx.req.set_uri_args]] should be used for this instead. For instance, Nginx config
37503751
37513752
<geshi lang="nginx">
@@ -4110,6 +4111,9 @@ is equivalent to
41104111
ngx.req.clear_header("X-Foo")
41114112
</geshi>
41124113
4114+
Note: this function throws a Lua error if <code>header_name</code> or
4115+
<code>header_value</code> contain unsafe characters (control characters).
4116+
41134117
== ngx.req.clear_header ==
41144118
41154119
'''syntax:''' ''ngx.req.clear_header(header_name)''
@@ -4398,12 +4402,8 @@ It is recommended that a coding style that combines this method call with the <c
43984402
43994403
Issue an <code>HTTP 301</code> or <code>302</code> redirection to <code>uri</code>.
44004404
4401-
Notice: the <code>uri</code> should not contains <code>\r</code> or <code>\n</code>, otherwise, the characters after <code>\r</code> or <code>\n</code> will be truncated, including the <code>\r</code> or <code>\n</code> bytes themself.
4402-
4403-
The <code>uri</code> argument will be truncated if it contains the
4404-
<code>\r</code> or <code>\n</code> characters. The truncated value will contain
4405-
all characters up to (and excluding) the first occurrence of <code>\r</code> or
4406-
<code>\n</code>.
4405+
Note: this function throws a Lua error if the <code>uri</code> argument
4406+
contains unsafe characters (control characters).
44074407
44084408
The optional <code>status</code> parameter specifies the HTTP status code to be used. The following status codes are supported right now:
44094409

‎src/ngx_http_lua_control.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,9 @@ ngx_http_lua_ngx_redirect(lua_State *L)
239239
"the headers");
240240
}
241241

242-
len = ngx_http_lua_safe_header_value_len(p, len);
242+
if (ngx_http_lua_check_header_safe(r, p, len) != NGX_OK) {
243+
return luaL_error(L, "attempt to use unsafe uri");
244+
}
243245

244246
uri = ngx_palloc(r->pool, len);
245247
if (uri == NULL) {

‎src/ngx_http_lua_headers_in.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -658,6 +658,12 @@ ngx_http_lua_set_input_header(ngx_http_request_t *r, ngx_str_t key,
658658

659659
dd("set header value: %.*s", (int) value.len, value.data);
660660

661+
if (ngx_http_lua_check_header_safe(r, key.data, key.len) != NGX_OK
662+
|| ngx_http_lua_check_header_safe(r, value.data, value.len) != NGX_OK)
663+
{
664+
return NGX_ERROR;
665+
}
666+
661667
hv.hash = ngx_hash_key_lc(key.data, key.len);
662668
hv.key = key;
663669

‎src/ngx_http_lua_headers_out.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -491,9 +491,11 @@ ngx_http_lua_set_output_header(ngx_http_request_t *r, ngx_http_lua_ctx_t *ctx,
491491

492492
dd("set header value: %.*s", (int) value.len, value.data);
493493

494-
key.len = ngx_http_lua_safe_header_value_len(key.data, key.len);
495-
496-
value.len = ngx_http_lua_safe_header_value_len(value.data, value.len);
494+
if (ngx_http_lua_check_header_safe(r, key.data, key.len) != NGX_OK
495+
|| ngx_http_lua_check_header_safe(r, value.data, value.len) != NGX_OK)
496+
{
497+
return NGX_ERROR;
498+
}
497499

498500
hv.hash = ngx_hash_key_lc(key.data, key.len);
499501
hv.key = key;

‎src/ngx_http_lua_uri.c

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717

1818
static int ngx_http_lua_ngx_req_set_uri(lua_State *L);
19+
static ngx_int_t ngx_http_lua_check_uri_safe(ngx_http_request_t *r,
20+
u_char *str, size_t len);
1921

2022

2123
void
@@ -55,6 +57,10 @@ ngx_http_lua_ngx_req_set_uri(lua_State *L)
5557
return luaL_error(L, "attempt to use zero-length uri");
5658
}
5759

60+
if (ngx_http_lua_check_uri_safe(r, p, len) != NGX_OK) {
61+
return luaL_error(L, "attempt to use unsafe uri");
62+
}
63+
5864
if (n == 2) {
5965

6066
luaL_checktype(L, 2, LUA_TBOOLEAN);
@@ -107,4 +113,57 @@ ngx_http_lua_ngx_req_set_uri(lua_State *L)
107113
return 0;
108114
}
109115

116+
117+
static ngx_inline ngx_int_t
118+
ngx_http_lua_check_uri_safe(ngx_http_request_t *r, u_char *str, size_t len)
119+
{
120+
size_t i, buf_len;
121+
u_char c;
122+
u_char *buf, *src = str;
123+
124+
/* %00-%1F, " ", %7F */
125+
126+
static uint32_t unsafe[] = {
127+
0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */
128+
129+
/* ?>=< ;:98 7654 3210 /.-, +*)( '&%$ #"! */
130+
0x00000001, /* 0000 0000 0000 0000 0000 0000 0000 0001 */
131+
132+
/* _^]\ [ZYX WVUT SRQP ONML KJIH GFED CBA@ */
133+
0x00000000, /* 0000 0000 0000 0000 0000 0000 0000 0000 */
134+
135+
/* ~}| {zyx wvut srqp onml kjih gfed cba` */
136+
0x80000000, /* 1000 0000 0000 0000 0000 0000 0000 0000 */
137+
138+
0x00000000, /* 0000 0000 0000 0000 0000 0000 0000 0000 */
139+
0x00000000, /* 0000 0000 0000 0000 0000 0000 0000 0000 */
140+
0x00000000, /* 0000 0000 0000 0000 0000 0000 0000 0000 */
141+
0x00000000 /* 0000 0000 0000 0000 0000 0000 0000 0000 */
142+
};
143+
144+
for (i = 0; i < len; i++, str++) {
145+
c = *str;
146+
if (unsafe[c >> 5] & (1 << (c & 0x1f))) {
147+
buf_len = ngx_http_lua_escape_log(NULL, src, len);
148+
buf = ngx_palloc(r->pool, buf_len);
149+
if (buf == NULL) {
150+
return NGX_ERROR;
151+
}
152+
153+
ngx_http_lua_escape_log(buf, src, len);
154+
155+
ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
156+
"unsafe byte \"0x%uxd\" in uri \"%*s\"",
157+
(unsigned) c, buf_len, buf);
158+
159+
ngx_pfree(r->pool, buf);
160+
161+
return NGX_ERROR;
162+
}
163+
}
164+
165+
return NGX_OK;
166+
}
167+
168+
110169
/* vi:set ft=c ts=4 sw=4 et fdm=marker: */

‎src/ngx_http_lua_util.c

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4261,4 +4261,123 @@ ngx_http_lua_set_sa_restart(ngx_log_t *log)
42614261
#endif
42624262

42634263

4264+
size_t
4265+
ngx_http_lua_escape_log(u_char *dst, u_char *src, size_t size)
4266+
{
4267+
size_t n;
4268+
u_char c;
4269+
static u_char hex[] = "0123456789ABCDEF";
4270+
4271+
static uint32_t escape[] = {
4272+
0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */
4273+
4274+
/* ?>=< ;:98 7654 3210 /.-, +*)( '&%$ #"! */
4275+
0x00000004, /* 0000 0000 0000 0000 0000 0000 0000 0100 */
4276+
4277+
/* _^]\ [ZYX WVUT SRQP ONML KJIH GFED CBA@ */
4278+
0x10000000, /* 0001 0000 0000 0000 0000 0000 0000 0000 */
4279+
4280+
/* ~}| {zyx wvut srqp onml kjih gfed cba` */
4281+
0x80000000, /* 1000 0000 0000 0000 0000 0000 0000 0000 */
4282+
4283+
0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */
4284+
0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */
4285+
0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */
4286+
0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */
4287+
};
4288+
4289+
if (dst == NULL) {
4290+
4291+
/* find the number of characters to be escaped */
4292+
4293+
n = 0;
4294+
4295+
while (size) {
4296+
c = *src;
4297+
if (escape[c >> 5] & (1 << (c & 0x1f))) {
4298+
n += 4;
4299+
4300+
} else {
4301+
n++;
4302+
}
4303+
4304+
src++;
4305+
size--;
4306+
}
4307+
4308+
return n;
4309+
}
4310+
4311+
while (size) {
4312+
c = *src;
4313+
if (escape[c >> 5] & (1 << (c & 0x1f))) {
4314+
*dst++ = '\\';
4315+
*dst++ = 'x';
4316+
*dst++ = hex[*src >> 4];
4317+
*dst++ = hex[*src & 0xf];
4318+
src++;
4319+
4320+
} else {
4321+
*dst++ = *src++;
4322+
}
4323+
4324+
size--;
4325+
}
4326+
4327+
return 0;
4328+
}
4329+
4330+
4331+
ngx_inline ngx_int_t
4332+
ngx_http_lua_check_header_safe(ngx_http_request_t *r, u_char *str, size_t len)
4333+
{
4334+
size_t i, buf_len;
4335+
u_char c;
4336+
u_char *buf, *src = str;
4337+
4338+
/* %00-%1F, %7F */
4339+
4340+
static uint32_t unsafe[] = {
4341+
0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */
4342+
4343+
/* ?>=< ;:98 7654 3210 /.-, +*)( '&%$ #"! */
4344+
0x00000000, /* 0000 0000 0000 0000 0000 0000 0000 0000 */
4345+
4346+
/* _^]\ [ZYX WVUT SRQP ONML KJIH GFED CBA@ */
4347+
0x00000000, /* 0000 0000 0000 0000 0000 0000 0000 0000 */
4348+
4349+
/* ~}| {zyx wvut srqp onml kjih gfed cba` */
4350+
0x80000000, /* 1000 0000 0000 0000 0000 0000 0000 0000 */
4351+
4352+
0x00000000, /* 0000 0000 0000 0000 0000 0000 0000 0000 */
4353+
0x00000000, /* 0000 0000 0000 0000 0000 0000 0000 0000 */
4354+
0x00000000, /* 0000 0000 0000 0000 0000 0000 0000 0000 */
4355+
0x00000000 /* 0000 0000 0000 0000 0000 0000 0000 0000 */
4356+
};
4357+
4358+
for (i = 0; i < len; i++, str++) {
4359+
c = *str;
4360+
if (unsafe[c >> 5] & (1 << (c & 0x1f))) {
4361+
buf_len = ngx_http_lua_escape_log(NULL, src, len);
4362+
buf = ngx_palloc(r->pool, buf_len);
4363+
if (buf == NULL) {
4364+
return NGX_ERROR;
4365+
}
4366+
4367+
ngx_http_lua_escape_log(buf, src, len);
4368+
4369+
ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
4370+
"unsafe byte \"0x%uxd\" in header \"%*s\"",
4371+
(unsigned) c, buf_len, buf);
4372+
4373+
ngx_pfree(r->pool, buf);
4374+
4375+
return NGX_ERROR;
4376+
}
4377+
}
4378+
4379+
return NGX_OK;
4380+
}
4381+
4382+
42644383
/* vi:set ft=c ts=4 sw=4 et fdm=marker: */

‎src/ngx_http_lua_util.h

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -241,20 +241,9 @@ void ngx_http_lua_cleanup_free(ngx_http_request_t *r,
241241
void ngx_http_lua_set_sa_restart(ngx_log_t *log);
242242
#endif
243243

244-
245-
static ngx_inline size_t
246-
ngx_http_lua_safe_header_value_len(u_char *str, size_t len)
247-
{
248-
size_t i;
249-
250-
for (i = 0; i < len; i++, str++) {
251-
if (*str == '\r' || *str == '\n') {
252-
return i;
253-
}
254-
}
255-
256-
return len;
257-
}
244+
size_t ngx_http_lua_escape_log(u_char *dst, u_char *src, size_t size);
245+
ngx_int_t ngx_http_lua_check_header_safe(ngx_http_request_t *r, u_char *str,
246+
size_t len);
258247

259248

260249
static ngx_inline void

0 commit comments

Comments
 (0)