Skip to content

Commit

Permalink
Strict validation of content length and chunk length. (#20)
Browse files Browse the repository at this point in the history
  • Loading branch information
ioquatix committed Jul 30, 2023
1 parent 329c2f4 commit e11fc16
Show file tree
Hide file tree
Showing 4 changed files with 140 additions and 5 deletions.
10 changes: 9 additions & 1 deletion lib/protocol/http1/body/chunked.rb
Expand Up @@ -35,12 +35,20 @@ def close(error = nil)
super
end

VALID_CHUNK_LENGTH = /\A[0-9a-fA-F]+\z/

# Follows the procedure outlined in https://tools.ietf.org/html/rfc7230#section-4.1.3
def read
return nil if @finished

length, extensions = read_line.split(";", 2)

unless length =~ VALID_CHUNK_LENGTH
raise BadRequest, "Invalid chunk length: #{length.dump}"
end

# It is possible this line contains chunk extension, so we use `to_i` to only consider the initial integral part:
length = read_line.to_i(16)
length = Integer(length, 16)

if length == 0
@finished = true
Expand Down
6 changes: 4 additions & 2 deletions lib/protocol/http1/connection.rb
Expand Up @@ -398,10 +398,12 @@ def read_tunnel_body
HEAD = "HEAD"
CONNECT = "CONNECT"

VALID_CONTENT_LENGTH = /\A\d+\z/

def extract_content_length(headers)
if content_length = headers.delete(CONTENT_LENGTH)
if length = Integer(content_length, exception: false) and length >= 0
yield length
if content_length =~ VALID_CONTENT_LENGTH
yield Integer(content_length, 10)
else
raise BadRequest, "Invalid content length: #{content_length}"
end
Expand Down
4 changes: 2 additions & 2 deletions test/protocol/http1/connection.rb
Expand Up @@ -208,15 +208,15 @@

with "HEAD" do
it "can read length of head response" do
body = client.read_response_body("HEAD", 200, {'content-length' => 3773})
body = client.read_response_body("HEAD", 200, {'content-length' => '3773'})

expect(body).to be_a ::Protocol::HTTP::Body::Head
expect(body.length).to be == 3773
expect(body.read).to be_nil
end

it "ignores zero length body" do
body = client.read_response_body("HEAD", 200, {'content-length' => 0})
body = client.read_response_body("HEAD", 200, {'content-length' => '0'})

expect(body).to be_nil
end
Expand Down
125 changes: 125 additions & 0 deletions test/protocol/http1/connection/bad.rb
@@ -0,0 +1,125 @@
# frozen_string_literal: true

# Released under the MIT License.
# Copyright, 2019-2023, by Samuel Williams.

require 'protocol/http1/connection'
require 'connection_context'

describe Protocol::HTTP1::Connection do
include_context ConnectionContext

This comment has been minimized.

Copy link
@dcar2121

dcar2121 Nov 18, 2023

curl 8.1.2 (x86_64-apple-darwin21.0) libcurl/8.1.2 (SecureTransport) LibreSSL/3.3.6 zlib/1.2.11 nghttp2/1.45.1
Release-Date: 2023-05-30
Protocols: dict file ftp ftps gopher gophers http https imap imaps ldap ldaps mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
Features: alt-svc AsynchDNS GSS-API HSTS HTTP2 HTTPS-proxy IPv6 Kerberos Largefile libz MultiSSL NTLM NTLM_WB SPNEGO SSL threadsafe UnixSockets

This comment has been minimized.

Copy link
@ioquatix

ioquatix Nov 18, 2023

Author Member

Hello 👋


def before
super

client.stream.write(input)
client.stream.close
end

with "invalid hexadecimal content-length" do
def input
<<~HTTP.gsub("\n", "\r\n")
POST / HTTP/1.1
Host: a.com
Content-Length: 0x10
Connection: close
0123456789abcdef
HTTP
end

it "should fail to parse the request body" do
expect do
server.read_request
end.to raise_exception(Protocol::HTTP1::BadRequest)
end
end

with "invalid +integer content-length" do
def input
<<~HTTP.gsub("\n", "\r\n")
POST / HTTP/1.1
Host: a.com
Content-Length: +16
Connection: close
0123456789abcdef
HTTP
end

it "should fail to parse the request body" do
expect do
server.read_request
end.to raise_exception(Protocol::HTTP1::BadRequest)
end
end

with "invalid -integer content-length" do
def input
<<~HTTP.gsub("\n", "\r\n")
POST / HTTP/1.1
Host: a.com
Content-Length: -16
Connection: close
0123456789abcdef
HTTP
end

it "should fail to parse the request body" do
expect do
server.read_request
end.to raise_exception(Protocol::HTTP1::BadRequest)
end
end

with "invalid hexidecimal chunk size" do
def input
<<~HTTP.gsub("\n", "\r\n")
POST / HTTP/1.1
Host: a.com
Transfer-Encoding: chunked
Connection: close
0x10
0123456789abcdef
0
HTTP
end

it "should fail to parse the request body" do
authority, method, target, version, headers, body = server.read_request

expect(body).to be_a(Protocol::HTTP1::Body::Chunked)

expect do
body.read
end.to raise_exception(Protocol::HTTP1::BadRequest)
end
end

with "invalid +integer chunk size" do
def input
<<~HTTP.gsub("\n", "\r\n")
POST / HTTP/1.1
Host: a.com
Transfer-Encoding: chunked
Connection: close
+10
0123456789abcdef
0
HTTP
end

it "should fail to parse the request body" do
authority, method, target, version, headers, body = server.read_request

expect(body).to be_a(Protocol::HTTP1::Body::Chunked)

expect do
body.read
end.to raise_exception(Protocol::HTTP1::BadRequest)
end
end
end

0 comments on commit e11fc16

Please sign in to comment.