Skip to content

Commit 48d594d

Browse files
petergoldsteinpetergoldstein
authored
Fixes #932 (#933)
Do some input sanitization for the meta protocol. Since this protocol uses raw test, it is possible to execute an injection attack against unsanitized inputs. This commit explicitly sanitizes the CAS arguments and the ttl passed to flush. I also added some missing tests for meta_arithmetic and fixed a few lints.
1 parent a8611e2 commit 48d594d

File tree

6 files changed

+133
-14
lines changed

6 files changed

+133
-14
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ Dalli Changelog
44
Unreleased
55
==========
66

7+
- Sanitize CAS inputs to ensure additional commands are not passed to memcached (xhzeem / petergoldstein)
8+
- Sanitize input to flush command to ensure additional commands are not passed to memcached (xhzeem / petergoldstein)
79
- Namespaces passed as procs are now evaluated every time, as opposed to just on initialization (nrw505)
810
- Fix missing require of uri in ServerConfigParser (adam12)
911
- Fix link to the CHANGELOG.md file in README.md (rud)

‎lib/dalli/protocol/meta.rb‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ def gat(key, ttl, options = nil)
4444
end
4545

4646
def touch(key, ttl)
47+
ttl = TtlSanitizer.sanitize(ttl)
4748
encoded_key, base64 = KeyRegularizer.encode(key)
4849
req = RequestFormatter.meta_get(key: encoded_key, ttl: ttl, value: false, base64: base64)
4950
write(req)

‎lib/dalli/protocol/meta/request_formatter.rb‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ def self.meta_set(key:, value:, bitflags: nil, cas: nil, ttl: nil, mode: :set, b
3131
cmd << ' c' unless %i[append prepend].include?(mode)
3232
cmd << ' b' if base64
3333
cmd << " F#{bitflags}" if bitflags
34-
cmd << " C#{cas}" if cas && !cas.zero?
34+
cmd << cas_string(cas)
3535
cmd << " T#{ttl}" if ttl
3636
cmd << " M#{mode_to_token(mode)}"
3737
cmd << ' q' if quiet
@@ -43,7 +43,7 @@ def self.meta_set(key:, value:, bitflags: nil, cas: nil, ttl: nil, mode: :set, b
4343
def self.meta_delete(key:, cas: nil, ttl: nil, base64: false, quiet: false)
4444
cmd = "md #{key}"
4545
cmd << ' b' if base64
46-
cmd << " C#{cas}" if cas && !cas.zero?
46+
cmd << cas_string(cas)
4747
cmd << " T#{ttl}" if ttl
4848
cmd << ' q' if quiet
4949
cmd + TERMINATOR
@@ -54,8 +54,9 @@ def self.meta_arithmetic(key:, delta:, initial:, incr: true, cas: nil, ttl: nil,
5454
cmd << ' b' if base64
5555
cmd << " D#{delta}" if delta
5656
cmd << " J#{initial}" if initial
57-
cmd << " C#{cas}" if cas && !cas.zero?
58-
cmd << " N#{ttl}" if ttl
57+
# Always set a TTL if an initial value is specified
58+
cmd << " N#{ttl || 0}" if ttl || initial
59+
cmd << cas_string(cas)
5960
cmd << ' q' if quiet
6061
cmd << " M#{incr ? 'I' : 'D'}"
6162
cmd + TERMINATOR
@@ -75,7 +76,7 @@ def self.version
7576

7677
def self.flush(delay: nil, quiet: false)
7778
cmd = +'flush_all'
78-
cmd << " #{delay}" if delay
79+
cmd << " #{parse_to_64_bit_int(delay, 0)}" if delay
7980
cmd << ' noreply' if quiet
8081
cmd + TERMINATOR
8182
end
@@ -102,6 +103,18 @@ def self.mode_to_token(mode)
102103
end
103104
end
104105
# rubocop:enable Metrics/MethodLength
106+
107+
def self.cas_string(cas)
108+
cas = parse_to_64_bit_int(cas, nil)
109+
cas.nil? || cas.zero? ? '' : " C#{cas}"
110+
end
111+
112+
def self.parse_to_64_bit_int(val, default)
113+
val.nil? ? nil : Integer(val)
114+
rescue ArgumentError
115+
# Sanitize to default if it isn't parsable as an integer
116+
default
117+
end
105118
end
106119
end
107120
end

‎test/integration/test_network.rb‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,17 @@
77
describe "using the #{p} protocol" do
88
describe 'assuming a bad network' do
99
it 'handle no server available' do
10+
dc = Dalli::Client.new 'localhost:19333'
1011
assert_raises Dalli::RingError, message: 'No server available' do
11-
dc = Dalli::Client.new 'localhost:19333'
1212
dc.get 'foo'
1313
end
1414
end
1515

1616
describe 'with a fake server' do
1717
it 'handle connection reset' do
1818
memcached_mock(->(sock) { sock.close }) do
19+
dc = Dalli::Client.new('localhost:19123')
1920
assert_raises Dalli::RingError, message: 'No server available' do
20-
dc = Dalli::Client.new('localhost:19123')
2121
dc.get('abc')
2222
end
2323
end
@@ -26,17 +26,17 @@
2626
it 'handle connection reset with unix socket' do
2727
socket_path = MemcachedMock::UNIX_SOCKET_PATH
2828
memcached_mock(->(sock) { sock.close }, :start_unix, socket_path) do
29+
dc = Dalli::Client.new(socket_path)
2930
assert_raises Dalli::RingError, message: 'No server available' do
30-
dc = Dalli::Client.new(socket_path)
3131
dc.get('abc')
3232
end
3333
end
3434
end
3535

3636
it 'handle malformed response' do
3737
memcached_mock(->(sock) { sock.write('123') }) do
38+
dc = Dalli::Client.new('localhost:19123')
3839
assert_raises Dalli::RingError, message: 'No server available' do
39-
dc = Dalli::Client.new('localhost:19123')
4040
dc.get('abc')
4141
end
4242
end
@@ -47,8 +47,8 @@
4747
sleep(0.6)
4848
sock.close
4949
}, :delayed_start) do
50+
dc = Dalli::Client.new('localhost:19123')
5051
assert_raises Dalli::RingError, message: 'No server available' do
51-
dc = Dalli::Client.new('localhost:19123')
5252
dc.get('abc')
5353
end
5454
end
@@ -59,8 +59,8 @@
5959
sleep(0.6)
6060
sock.write('giraffe')
6161
}) do
62+
dc = Dalli::Client.new('localhost:19123')
6263
assert_raises Dalli::RingError, message: 'No server available' do
63-
dc = Dalli::Client.new('localhost:19123')
6464
dc.get('abc')
6565
end
6666
end

‎test/protocol/meta/test_request_formatter.rb‎

Lines changed: 104 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,28 @@
7777
Dalli::Protocol::Meta::RequestFormatter.meta_set(key: key, value: val, bitflags: bitflags, cas: cas)
7878
end
7979

80+
it 'excludes CAS if set to 0' do
81+
assert_equal "ms #{key} #{val.bytesize} c F#{bitflags} MS\r\n#{val}\r\n",
82+
Dalli::Protocol::Meta::RequestFormatter.meta_set(key: key, value: val, bitflags: bitflags, cas: 0)
83+
end
84+
85+
it 'excludes non-numeric CAS values' do
86+
assert_equal "ms #{key} #{val.bytesize} c F#{bitflags} MS\r\n#{val}\r\n",
87+
Dalli::Protocol::Meta::RequestFormatter.meta_set(key: key, value: val, bitflags: bitflags,
88+
cas: "\nset importantkey 1 1000 8\ninjected")
89+
end
90+
8091
it 'sets the quiet mode if configured' do
8192
assert_equal "ms #{key} #{val.bytesize} c F#{bitflags} MS q\r\n#{val}\r\n",
8293
Dalli::Protocol::Meta::RequestFormatter.meta_set(key: key, value: val, bitflags: bitflags,
8394
quiet: true)
8495
end
96+
97+
it 'sets the base64 mode if configured' do
98+
assert_equal "ms #{key} #{val.bytesize} c b F#{bitflags} MS\r\n#{val}\r\n",
99+
Dalli::Protocol::Meta::RequestFormatter.meta_set(key: key, value: val, bitflags: bitflags,
100+
base64: true)
101+
end
85102
end
86103

87104
describe 'meta_delete' do
@@ -93,7 +110,7 @@
93110
Dalli::Protocol::Meta::RequestFormatter.meta_delete(key: key)
94111
end
95112

96-
it 'returns incorporates CAS when passed cas' do
113+
it 'incorporates CAS when passed cas' do
97114
assert_equal "md #{key} C#{cas}\r\n",
98115
Dalli::Protocol::Meta::RequestFormatter.meta_delete(key: key, cas: cas)
99116
end
@@ -102,6 +119,87 @@
102119
assert_equal "md #{key} q\r\n",
103120
Dalli::Protocol::Meta::RequestFormatter.meta_delete(key: key, quiet: true)
104121
end
122+
123+
it 'excludes CAS when set to 0' do
124+
assert_equal "md #{key}\r\n",
125+
Dalli::Protocol::Meta::RequestFormatter.meta_delete(key: key, cas: 0)
126+
end
127+
128+
it 'excludes non-numeric CAS values' do
129+
assert_equal "md #{key}\r\n",
130+
Dalli::Protocol::Meta::RequestFormatter.meta_delete(key: key,
131+
cas: "\nset importantkey 1 1000 8\ninjected")
132+
end
133+
134+
it 'sets the base64 mode if configured' do
135+
assert_equal "md #{key} b\r\n",
136+
Dalli::Protocol::Meta::RequestFormatter.meta_delete(key: key, base64: true)
137+
end
138+
end
139+
140+
describe 'meta_arithmetic' do
141+
let(:key) { SecureRandom.hex(4) }
142+
let(:delta) { rand(500..999) }
143+
let(:initial) { rand(500..999) }
144+
let(:cas) { rand(500..999) }
145+
let(:ttl) { rand(500..999) }
146+
147+
it 'returns the expected string with the default N flag when passed non-nil key, delta, and initial' do
148+
assert_equal "ma #{key} v D#{delta} J#{initial} N0 MI\r\n",
149+
Dalli::Protocol::Meta::RequestFormatter.meta_arithmetic(key: key, delta: delta, initial: initial)
150+
end
151+
152+
it 'excludes the J and N flags when initial is nil and ttl is not set' do
153+
assert_equal "ma #{key} v D#{delta} MI\r\n",
154+
Dalli::Protocol::Meta::RequestFormatter.meta_arithmetic(key: key, delta: delta, initial: nil)
155+
end
156+
157+
it 'omits the D flag is delta is nil' do
158+
assert_equal "ma #{key} v J#{initial} N0 MI\r\n",
159+
Dalli::Protocol::Meta::RequestFormatter.meta_arithmetic(key: key, delta: nil, initial: initial)
160+
end
161+
162+
it 'uses ttl for the N flag when ttl passed explicitly along with an initial value' do
163+
assert_equal "ma #{key} v D#{delta} J#{initial} N#{ttl} MI\r\n",
164+
Dalli::Protocol::Meta::RequestFormatter.meta_arithmetic(key: key, delta: delta, initial: initial,
165+
ttl: ttl)
166+
end
167+
168+
it 'incorporates CAS when passed cas' do
169+
assert_equal "ma #{key} v D#{delta} J#{initial} N0 C#{cas} MI\r\n",
170+
Dalli::Protocol::Meta::RequestFormatter.meta_arithmetic(key: key, delta: delta, initial: initial,
171+
cas: cas)
172+
end
173+
174+
it 'excludes CAS when CAS is set to 0' do
175+
assert_equal "ma #{key} v D#{delta} J#{initial} N0 MI\r\n",
176+
Dalli::Protocol::Meta::RequestFormatter.meta_arithmetic(key: key, delta: delta, initial: initial,
177+
cas: 0)
178+
end
179+
180+
it 'includes the N flag when ttl passed explicitly with a nil initial value' do
181+
assert_equal "ma #{key} v D#{delta} N#{ttl} MI\r\n",
182+
Dalli::Protocol::Meta::RequestFormatter.meta_arithmetic(key: key, delta: delta, initial: nil,
183+
ttl: ttl)
184+
end
185+
186+
it 'swaps from MI to MD when the incr value is explicitly false' do
187+
assert_equal "ma #{key} v D#{delta} J#{initial} N0 MD\r\n",
188+
Dalli::Protocol::Meta::RequestFormatter.meta_arithmetic(key: key, delta: delta, initial: initial,
189+
incr: false)
190+
end
191+
192+
it 'includes the quiet flag when specified' do
193+
assert_equal "ma #{key} v D#{delta} J#{initial} N0 q MI\r\n",
194+
Dalli::Protocol::Meta::RequestFormatter.meta_arithmetic(key: key, delta: delta, initial: initial,
195+
quiet: true)
196+
end
197+
198+
it 'sets the base64 mode if configured' do
199+
assert_equal "ma #{key} v b D#{delta} J#{initial} N0 MI\r\n",
200+
Dalli::Protocol::Meta::RequestFormatter.meta_arithmetic(key: key, delta: delta, initial: initial,
201+
base64: true)
202+
end
105203
end
106204

107205
describe 'meta_noop' do
@@ -130,6 +228,11 @@
130228
assert_equal "flush_all #{delay}\r\n", Dalli::Protocol::Meta::RequestFormatter.flush(delay: delay)
131229
end
132230

231+
it 'santizes the delay argument' do
232+
delay = "\nset importantkey 1 1000 8\ninjected"
233+
assert_equal "flush_all 0\r\n", Dalli::Protocol::Meta::RequestFormatter.flush(delay: delay)
234+
end
235+
133236
it 'adds noreply with a delay and quiet argument' do
134237
delay = rand(1000..1999)
135238
assert_equal "flush_all #{delay} noreply\r\n",

‎test/test_rack_session.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,15 @@
5454
let(:incrementor) { Rack::Lint.new(incrementor_proc) }
5555

5656
it 'faults on no connection' do
57+
rsd = Rack::Session::Dalli.new(incrementor, memcache_server: 'nosuchserver')
5758
assert_raises Dalli::RingError do
58-
rsd = Rack::Session::Dalli.new(incrementor, memcache_server: 'nosuchserver')
5959
rsd.data.with { |c| c.set('ping', '') }
6060
end
6161
end
6262

6363
it 'connects to existing server' do
64+
rsd = Rack::Session::Dalli.new(incrementor, namespace: 'test:rack:session')
6465
assert_silent do
65-
rsd = Rack::Session::Dalli.new(incrementor, namespace: 'test:rack:session')
6666
rsd.data.with { |c| c.set('ping', '') }
6767
end
6868
end

0 commit comments

Comments
 (0)