Skip to content

Commit

Permalink
Fix TOCTOU bug
Browse files Browse the repository at this point in the history
This fixes the time of check to time of use (TOCTOU) bug reported by Sajjad
Hashemian.
  • Loading branch information
jtdowney committed May 4, 2018
1 parent 9495eb0 commit 4068228
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 6 deletions.
12 changes: 11 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,18 @@ To install this gem onto your local machine, run `bundle exec rake install`. To

Bug reports and pull requests are welcome on GitHub at https://github.com/jtdowney/private_address_check. This project is intended to be a safe, welcoming space for collaboration, and contributors are expected to adhere to the [Contributor Covenant](http://contributor-covenant.org) code of conduct.

## Security

If you've found a security issue in `private_address_check`, please reach out to @jtdowney via email to report.

### Time of check to time of use

A library like `private_address_check` is going to be easily susceptible to attacks like [time of check to time of use](https://en.wikipedia.org/wiki/Time_of_check_to_time_of_use). DNS entries with a TTL of 0 can trigger this case where the initial resolution is a public address by the subsequent resolution is a private address. There are some possible defenses and workarounds:

- Use the TCPSocket extension in this library which checks the address the socket uses. This is most useful if your system is built on native Ruby like Net::HTTP.
- Use a feature like the `resolve` capability in curl and [curb](https://www.rubydoc.info/github/taf2/curb/Curl/Easy#resolve=-instance_method) to force the resolution to a pre-checked IP address.
- Implement your own caching DNS resolver with something like dnsmasq or unbound. These tools let you set a minimum cache time that can override the TTL of 0.

## License

The gem is available as open source under the terms of the [MIT License](http://opensource.org/licenses/MIT).

7 changes: 3 additions & 4 deletions lib/private_address_check/tcpsocket_ext.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,10 @@ def only_public_connections
TCPSocket.class_eval do
alias initialize_without_private_address_check initialize

def initialize(remote_host, remote_port, local_host = nil, local_port = nil)
if Thread.current[:private_address_check] && PrivateAddressCheck.resolves_to_private_address?(remote_host)
def initialize(*args)
initialize_without_private_address_check(*args)
if Thread.current[:private_address_check] && PrivateAddressCheck.resolves_to_private_address?(remote_address.ip_address)
raise PrivateAddressCheck::PrivateConnectionAttemptedError
end

initialize_without_private_address_check(remote_host, remote_port, local_host, local_port)
end
end
6 changes: 5 additions & 1 deletion test/private_address_check/tcpsocket_ext_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,15 @@

class TCPSocketExtTest < Minitest::Test
def test_private_address
server = TCPServer.new(63453)
thread = Thread.start { server.accept }
assert_raises PrivateAddressCheck::PrivateConnectionAttemptedError do
PrivateAddressCheck.only_public_connections do
TCPSocket.new("localhost", 80)
TCPSocket.new("localhost", 63453)
end
end
ensure
thread.exit if thread
end

def test_public_address
Expand Down

1 comment on commit 4068228

@reedloden
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been assigned CVE-2018-3759.

Please sign in to comment.