Skip to content

Commit f01721a

Browse files
committed
Use buffer length instead of capacity in hash/mac calculation
1 parent 2494d46 commit f01721a

File tree

4 files changed

+118
-20
lines changed

4 files changed

+118
-20
lines changed
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
# Copyright (c) 2023, 2023, Oracle and/or its affiliates. All rights reserved.
2+
# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
3+
#
4+
# The Universal Permissive License (UPL), Version 1.0
5+
#
6+
# Subject to the condition set forth below, permission is hereby granted to any
7+
# person obtaining a copy of this software, associated documentation and/or
8+
# data (collectively the "Software"), free of charge and under any and all
9+
# copyright rights in the Software, and any and all patent rights owned or
10+
# freely licensable by each licensor hereunder covering either (i) the
11+
# unmodified Software as contributed to or provided by such licensor, or (ii)
12+
# the Larger Works (as defined below), to deal in both
13+
#
14+
# (a) the Software, and
15+
#
16+
# (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
17+
# one is included with the Software each a "Larger Work" to which the Software
18+
# is contributed by such licensors),
19+
#
20+
# without restriction, including without limitation the rights to copy, create
21+
# derivative works of, display, perform, and distribute the Software and make,
22+
# use, sell, offer for sale, import, export, have made, and have sold the
23+
# Software and the Larger Work(s), and to sublicense the foregoing rights on
24+
# either these or other terms.
25+
#
26+
# This license is subject to the following condition:
27+
#
28+
# The above copyright notice and either this complete permission notice or at a
29+
# minimum a reference to the UPL must be included in all copies or substantial
30+
# portions of the Software.
31+
#
32+
# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
33+
# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
34+
# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
35+
# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
36+
# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
37+
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
38+
# SOFTWARE.
39+
40+
import hashlib
41+
import hmac
42+
import unittest
43+
44+
45+
class HashlibTest(unittest.TestCase):
46+
47+
def test_messagedigest_update_after_digest(self):
48+
sha1 = hashlib.sha1()
49+
sha1.update(b'a')
50+
self.assertEqual('86f7e437faa5a7fce15d1ddcb9eaeaea377667b8', sha1.hexdigest())
51+
sha1.update(b'b')
52+
self.assertEqual('da23614e02469a0d7c7bd1bdab5c9c474b1904dc', sha1.hexdigest())
53+
54+
def test_mac_update_after_digest(self):
55+
hm = hmac.new(b'key', digestmod=hashlib.sha256)
56+
hm.update(b'a')
57+
self.assertEqual('780c3db4ce3de5b9e55816fba98f590631d96c075271b26976238d5f4444219b', hm.hexdigest())
58+
hm.update(b'b')
59+
self.assertEqual('5c1c0c948cbef5ad7d191e46e5901fa0395f85f694fa4633f665a1a637017b65', hm.hexdigest())
60+
61+
def test_messagedigest_buffer_length_in_constructor(self):
62+
sha1 = hashlib.sha1(self._get_buffer())
63+
self.assertEqual('86f7e437faa5a7fce15d1ddcb9eaeaea377667b8', sha1.hexdigest())
64+
65+
def test_messagedigest_buffer_length_in_update(self):
66+
sha1 = hashlib.sha1()
67+
sha1.update(self._get_buffer())
68+
self.assertEqual('86f7e437faa5a7fce15d1ddcb9eaeaea377667b8', sha1.hexdigest())
69+
70+
def test_mac_buffer_length_in_constructor(self):
71+
hm = hmac.new(b'key', self._get_buffer(), digestmod=hashlib.sha256)
72+
self.assertEqual('780c3db4ce3de5b9e55816fba98f590631d96c075271b26976238d5f4444219b', hm.hexdigest())
73+
74+
def test_mac_buffer_length_in_update(self):
75+
hm = hmac.new(b'key', digestmod=hashlib.sha256)
76+
hm.update(self._get_buffer())
77+
self.assertEqual('780c3db4ce3de5b9e55816fba98f590631d96c075271b26976238d5f4444219b', hm.hexdigest())
78+
79+
def test_mac_key_length(self):
80+
hm = hmac.new(self._get_buffer(), b'data', digestmod=hashlib.sha256)
81+
self.assertEqual('c449f6626bf7f997cda786d07895f086c2fa18eab25b1c08c4de66a5d46a2a08', hm.hexdigest())
82+
83+
@staticmethod
84+
def _get_buffer():
85+
ba = bytearray(b'ab')
86+
ba.remove(ord('b'))
87+
# The capacity of ba should now be different from its length.
88+
# Also, the rest of the buffer is not filled with binary zeroes - this is important for
89+
# test_mac_key_length because according to RFC 2104, the key is padded with zeroes anyway.
90+
# So providing a buffer with capacity > length, but with zero padding as the key argument to HMAC
91+
# would not trigger the bug.
92+
return ba
93+
94+
95+
if __name__ == '__main__':
96+
unittest.main()

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/hashlib/DigestObject.java

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -182,21 +182,21 @@ private PythonBuiltinClassType determineMainDigestType() {
182182
* calculate the digest on a clone, but that does not need to be supported. If it is not, then
183183
* we calculate the digest normally, but we must prevent any further updates.
184184
*
185-
* @see #wasReset(), {@link #update(byte[])}
185+
* @see #wasReset(), {@link #update(byte[], int)}
186186
*/
187187
abstract byte[] digest();
188188

189189
/**
190190
* @return true if the digest has already been calculated and the underlying implementation does
191191
* not support cloning, in which case this object can no longer be
192-
* {@linkplain #update(byte[]) updated}
192+
* {@linkplain #update(byte[], int) updated}
193193
*/
194194
abstract boolean wasReset();
195195

196196
/**
197197
* Must not be called if {@link #wasReset()} returns true.
198198
*/
199-
abstract void update(byte[] data);
199+
abstract void update(byte[] data, int length);
200200

201201
abstract DigestObject copy(PythonObjectFactory factory) throws CloneNotSupportedException;
202202

@@ -207,8 +207,8 @@ final String getAlgorithm() {
207207
}
208208

209209
/**
210-
* Ensures that {@link #update(byte[])} is not called after {@link #digest()} if cloning is not
211-
* supported. Also caches the digest and ensures that the cache is cleared on update.
210+
* Ensures that {@link #update(byte[], int)} is not called after {@link #digest()} if cloning is
211+
* not supported. Also caches the digest and ensures that the cache is cleared on update.
212212
*/
213213
private abstract static class DigestObjectBase extends DigestObject {
214214
private byte[] cachedDigest = null;
@@ -237,19 +237,19 @@ final byte[] digest() {
237237
}
238238

239239
@Override
240-
final void update(byte[] data) {
240+
final void update(byte[] data, int length) {
241241
if (wasReset) {
242242
throw CompilerDirectives.shouldNotReachHere("update() called after digest() on an implementation the does not support clone()");
243243
}
244244
cachedDigest = null;
245-
doUpdate(data);
245+
doUpdate(data, length);
246246
}
247247

248248
abstract byte[] calculateDigestOnClone() throws CloneNotSupportedException;
249249

250250
abstract byte[] calculateDigest();
251251

252-
abstract void doUpdate(byte[] data);
252+
abstract void doUpdate(byte[] data, int length);
253253
}
254254

255255
private static final class MessageDigestObject extends DigestObjectBase {
@@ -280,8 +280,8 @@ byte[] calculateDigest() {
280280

281281
@Override
282282
@TruffleBoundary
283-
void doUpdate(byte[] data) {
284-
digest.update(data);
283+
void doUpdate(byte[] data, int length) {
284+
digest.update(data, 0, length);
285285
}
286286

287287
@Override
@@ -319,8 +319,8 @@ byte[] calculateDigest() {
319319

320320
@Override
321321
@TruffleBoundary
322-
void doUpdate(byte[] data) {
323-
mac.update(data);
322+
void doUpdate(byte[] data, int length) {
323+
mac.update(data, 0, length);
324324
}
325325

326326
@Override

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/hashlib/DigestObjectBuiltins.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ PNone update(VirtualFrame frame, DigestObject self, Object buffer,
127127
raise(PythonBuiltinClassType.ValueError, ErrorMessages.UPDATING_FINALIZED_DIGEST_IS_NOT_SUPPORTED);
128128
}
129129
try {
130-
self.update(bufferLib.getInternalOrCopiedByteArray(buffer));
130+
self.update(bufferLib.getInternalOrCopiedByteArray(buffer), bufferLib.getBufferLength(buffer));
131131
} finally {
132132
bufferLib.release(buffer, frame, this);
133133
}

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/hashlib/HashlibModuleBuiltins.java

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,8 @@ Object hmacNew(@SuppressWarnings("unused") PythonModule self, Object keyObj, Obj
314314
}
315315
try {
316316
byte[] msgBytes = msg == null ? null : bufferLib.getInternalOrCopiedByteArray(msg);
317-
Mac mac = createMac(digestmod, bufferLib.getInternalOrCopiedByteArray(key), msgBytes);
317+
int msgLen = msg == null ? 0 : bufferLib.getBufferLength(msg);
318+
Mac mac = createMac(digestmod, bufferLib.getInternalOrCopiedByteArray(key), bufferLib.getBufferLength(key), msgBytes, msgLen);
318319
return factory().createDigestObject(PythonBuiltinClassType.HashlibHmac, castJStr.execute(concatStr.execute(HMAC_PREFIX, digestmod, TS_ENCODING, true)), mac);
319320
} catch (InvalidKeyException | NoSuchAlgorithmException e) {
320321
throw raise(PythonBuiltinClassType.UnsupportedDigestmodError, e);
@@ -330,14 +331,14 @@ Object hmacNew(@SuppressWarnings("unused") PythonModule self, Object keyObj, Obj
330331
}
331332

332333
@TruffleBoundary
333-
static Mac createMac(TruffleString digest, byte[] key, byte[] msg) throws NoSuchAlgorithmException, InvalidKeyException {
334+
static Mac createMac(TruffleString digest, byte[] key, int keyLen, byte[] msg, int msgLen) throws NoSuchAlgorithmException, InvalidKeyException {
334335
String inputName = digest.toJavaStringUncached().toLowerCase();
335336
String algorithm = "hmac" + NAME_MAPPINGS.getOrDefault(inputName, inputName);
336-
SecretKeySpec secretKeySpec = new SecretKeySpec(key, algorithm);
337+
SecretKeySpec secretKeySpec = new SecretKeySpec(key, 0, keyLen, algorithm);
337338
Mac mac = Mac.getInstance(algorithm);
338339
mac.init(secretKeySpec);
339340
if (msg != null) {
340-
mac.update(msg);
341+
mac.update(msg, 0, msgLen);
341342
}
342343
return mac;
343344
}
@@ -361,9 +362,10 @@ Object create(VirtualFrame frame, PythonBuiltinClassType type, String pythonName
361362
}
362363
try {
363364
byte[] bytes = buffer == null ? null : bufferLib.getInternalOrCopiedByteArray(buffer);
365+
int bytesLen = buffer == null ? 0 : bufferLib.getBufferLength(buffer);
364366
MessageDigest digest;
365367
try {
366-
digest = createDigest(javaName, bytes);
368+
digest = createDigest(javaName, bytes, bytesLen);
367369
} catch (NoSuchAlgorithmException e) {
368370
throw raise.raise(PythonBuiltinClassType.UnsupportedDigestmodError, e);
369371
}
@@ -376,10 +378,10 @@ Object create(VirtualFrame frame, PythonBuiltinClassType type, String pythonName
376378
}
377379

378380
@TruffleBoundary
379-
private static MessageDigest createDigest(String name, byte[] bytes) throws NoSuchAlgorithmException {
381+
private static MessageDigest createDigest(String name, byte[] bytes, int bytesLen) throws NoSuchAlgorithmException {
380382
MessageDigest digest = MessageDigest.getInstance(name);
381383
if (bytes != null) {
382-
digest.update(bytes);
384+
digest.update(bytes, 0, bytesLen);
383385
}
384386
return digest;
385387
}

0 commit comments

Comments
 (0)