Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: crystal-lang/crystal
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 2e00b8f7a779
Choose a base ref
...
head repository: crystal-lang/crystal
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 99c3d2b624db
Choose a head ref
  • 2 commits
  • 2 files changed
  • 2 contributors

Commits on Oct 31, 2017

  1. ENV#[]=: Disallow null bytes in key and value 

    Before this, a call to `ENV[k] = v` would not check for NUL-bytes.
`setenv(3)` would then take the key and value, and as it doesn't know
about the embedded NUL-byte(s) in the key and/or value, would truncate
at the NUL-byte.

This may not sound too terrible at first.  But it can turn into a
security issue:

```cr
from_user = "DO-SOMETHING-BAD\0GOOD" # Received from user input
if from_user.ends_with?("GOOD") # Will match
  ENV["FOO"] = from_user # Will set "FOO" to "DO-SOMETHING-BAD"
end
```
    @Papierkorb
    Papierkorb committed Oct 31, 2017
    Copy the full SHA
    81a4c29 View commit details

Commits on Nov 1, 2017

  1. Merge pull request #5216 from Papierkorb/disallow-nul-in-setenv 

    ENV#[]=: Disallow null bytes in key and value
    @asterite
    asterite authored Nov 1, 2017
    Copy the full SHA
    99c3d2b View commit details
Showing with 27 additions and 1 deletion.
  1. +20 −0 spec/std/env_spec.cr
  2. +7 −1 src/env.cr
20 changes: 20 additions & 0 deletions spec/std/env_spec.cr
View file
Original file line number Diff line number Diff line change
@@ -50,6 +50,26 @@ describe "ENV" do
[1, 2].each { |i| ENV.values.should contain("SOMEVALUE_#{i}") }
end

describe "[]=" do
it "disallows NUL-bytes in key" do
expect_raises(ArgumentError, "Key contains null byte") do
ENV["FOO\0BAR"] = "something"
end
end

it "disallows NUL-bytes in key if value is nil" do
expect_raises(ArgumentError, "Key contains null byte") do
ENV["FOO\0BAR"] = nil
end
end

it "disallows NUL-bytes in value" do
expect_raises(ArgumentError, "Value contains null byte") do
ENV["FOO"] = "BAR\0BAZ"
end
end
end

describe "fetch" do
it "fetches with one argument" do
ENV["1"] = "2"
8 changes: 7 additions & 1 deletion src/env.cr
View file
Original file line number Diff line number Diff line change
@@ -32,10 +32,16 @@ module ENV
# Overwrites existing environment variable if already present.
# Returns *value* if successful, otherwise raises an exception.
# If *value* is `nil`, the environment variable is deleted.
#
# If *key* or *value* contains a null-byte an `ArgumentError` is raised.
def self.[]=(key : String, value : String?)
raise ArgumentError.new("Key contains null byte") if key.byte_index(0)

if value
raise ArgumentError.new("Value contains null byte") if value.byte_index(0)

if LibC.setenv(key, value, 1) != 0
raise Errno.new("Error setting environment variable \"#{key}\"")
raise Errno.new("Error setting environment variable #{key.inspect}")
end
else
LibC.unsetenv(key)