Ticket #1367 (closed defect: fixed)

Opened 18 months ago

Last modified 18 months ago

Varnish crashes if GET consists from whitespaces only

Reported by: elurin Owned by: Poul-Henning Kamp <phk@…>
Priority: normal Milestone:
Component: build Version: 3.0.4
Severity: normal Keywords:
Cc:

Description

Description: If GET parameter consists only from whitespaces a varnish child crashes. Unfortunately, sometimes we have bad queries from clients with whitespaces. Steps to reproduce:

  1. Create simple http request with several whitespaces after GET.
    vec01g:/etc/varnish# telnet localhost 80
    Trying ::1...
    Connected to localhost.localdomain.
    Escape character is '^]'.
    GET       
    Host: example.com
    
    Connection closed by foreign host.
    
    
  1. Varnish crashes with trace:
Oct 29 17:43:23 vec01g Condition((t.b) != 0) not true.
Oct 29 17:43:23 vec01g thread = (cache-worker)
Oct 29 17:43:23 vec01g ident = Linux,3.8.0-11-na,x86_64,-smalloc,-smalloc,-hcritbit,epoll
Oct 29 17:43:23 vec01g Backtrace:
Oct 29 17:43:23 vec01g 0x430c28: /usr/sbin/varnishd() [0x430c28]
Oct 29 17:43:23 vec01g 0x42d555: /usr/sbin/varnishd() [0x42d555]
Oct 29 17:43:23 vec01g 0x42dd79: /usr/sbin/varnishd(http_FilterHeader+0x89) [0x42dd79]
Oct 29 17:43:23 vec01g 0x41a755: /usr/sbin/varnishd(CNT_Session+0x7f5) [0x41a755]
Oct 29 17:43:23 vec01g 0x432971: /usr/sbin/varnishd() [0x432971]
Oct 29 17:43:23 vec01g 0x7fd813a349ca: /lib/libpthread.so.0(+0x69ca) [0x7fd813a349ca]
Oct 29 17:43:23 vec01g 0x7fd81379170d: /lib/libc.so.6(clone+0x6d) [0x7fd81379170d]
Oct 29 17:43:23 vec01g sp = 0x7fd740489008 {
Oct 29 17:43:23 vec01g fd = 3, id = 3, xid = 1901155339,
Oct 29 17:43:23 vec01g client = ::1 49391,
Oct 29 17:43:23 vec01g step = STP_PIPE,
Oct 29 17:43:23 vec01g handling = hash,
Oct 29 17:43:23 vec01g err_code = 413, err_reason = (null),
Oct 29 17:43:23 vec01g restarts = 1, esi_level = 0
Oct 29 17:43:23 vec01g flags =.
Oct 29 17:43:23 vec01g bodystatus = 4
Oct 29 17:43:23 vec01g ws = 0x7fd740489080 {.
Oct 29 17:43:23 vec01g id = "sess",
Oct 29 17:43:23 vec01g {s,f,r,e} = {0x7fd740489c78,+408,(nil),+262144},
Oct 29 17:43:23 vec01g },
Oct 29 17:43:23 vec01g http[req] = {
Oct 29 17:43:23 vec01g ws = 0x7fd740489080[sess]
Oct 29 17:43:23 vec01g "GET",
Oct 29 17:43:23 vec01g "
Oct 29 17:43:23 vec01g Host: example.com
Oct 29 17:43:23 vec01g host: example-internal.com",
Oct 29 17:43:23 vec01g "host: example-internal.com",
Oct 29 17:43:23 vec01g },
Oct 29 17:43:23 vec01g worker = 0x7fd74116da90 {
Oct 29 17:43:23 vec01g ws = 0x7fd74116dcc8 {.
Oct 29 17:43:23 vec01g id = "wrk",
Oct 29 17:43:23 vec01g {s,f,r,e} = {0x7fd741155a20,0x7fd741155a20,(nil),+65536},
Oct 29 17:43:23 vec01g },
Oct 29 17:43:23 vec01g http[bereq] = {
Oct 29 17:43:23 vec01g ws = 0x7fd74116dcc8[wrk]
Oct 29 17:43:23 vec01g "GET",
Oct 29 17:43:23 vec01g "
Oct 29 17:43:23 vec01g Host: example.com
Oct 29 17:43:23 vec01g host: example-internal.com",
Oct 29 17:43:23 vec01g },
Oct 29 17:43:23 vec01g },
Oct 29 17:43:23 vec01g vcl = {
Oct 29 17:43:23 vec01g srcname = {
Oct 29 17:43:23 vec01g "input",
Oct 29 17:43:23 vec01g "Default",
Oct 29 17:43:23 vec01g "/etc/varnish/ban.list",
Oct 29 17:43:23 vec01g "/etc/varnish/backends-load.vcl",
Oct 29 17:43:23 vec01g },
Oct 29 17:43:23 vec01g },
Oct 29 17:43:23 vec01g },
Oct 29 13:43:23 vec01g varnishd[4942]: Child cleanup complete

Varnish version: 3.0.4 Ident: ident = Linux,3.8.0-11-na,x86_64,-smalloc,-smalloc,-hcritbit,epoll

Of course, we can normalize queries in the vec_recv section with "if (req.url !~ "/") {error 400;}" match,but this is a wooden leg.

Attachments

1367.diff Download (368 bytes) - added by martin 18 months ago.

Change History

comment:1 Changed 18 months ago by lkarsten

I'm not able to reproduce this on my system with stock 3.0.4.

Can you please provide your VCL? (either here or to security@…)

comment:2 Changed 18 months ago by elurin

I've reproduced this problem with your build 3.0.4-1~lucid for Ubuntu 10.04 (lucid) from your official repository. Also, I've sent vcl to your email. Thank you!

comment:3 Changed 18 months ago by lkarsten

Short term workaround to avoid a panic:

sub vcl_error {
    if (obj.status >= 400 && obj.status < 500) { return(deliver); }
}

append this before any other vcl_error code is run.

comment:4 Changed 18 months ago by elurin

Thank you, it works:

vec01g.load:/var/log# telnet localhost 80
Trying ::1...
Connected to localhost.localdomain.
Escape character is '^]'.
GET    
Host: example.com

HTTP/1.1 413 Request Entity Too Large
Accept-Ranges: bytes
Date: Tue, 29 Oct 2013 15:57:14 GMT
Connection: close

Connection closed by foreign host.
vec01g.load:/var/log# 

Changed 18 months ago by martin

comment:5 Changed 18 months ago by martin

Attached diff seems to fix the code issue.

I can't see any reason to not do the 400 return which causes Varnish to hang up when the URL is missing.

Test case still missing though.

Martin

comment:6 Changed 18 months ago by Poul-Henning Kamp <phk@…>

  • Status changed from new to closed
  • Owner set to Poul-Henning Kamp <phk@…>
  • Resolution set to fixed

In [9c9a9904bdb56b62017f338baf9c8e906b88dcac]:

Make up our mind: Any req.* we receive from the client with fundamental
trouble gets failed back without VCL involvement.

Fixes #1367

comment:7 Changed 18 months ago by Martin Blix Grydeland <martin@…>

In [4bd5b7991bf602a6c46dd0d65fc04d4b8d9667a6]:

Make up our mind: Any req.* we receive from the client with fundamental
trouble gets failed back without VCL involvement.

Fixes #1367

Note: See TracTickets for help on using tickets.