Skip to contentSkip to conversation

libblkid: prevent OOB access while probing HFS+

PATCH Applied Tobias Stoeckmann 2 participants 3 messages Mar 15, 2017 – Mar 24, 2017 Util-Linux package

lore
Share & tools
lore

Tobias Stoeckmann

author
#0 lore raw

PATCH 1/1 libblkid: prevent OOB access while probing HFS+

It is possible to perform out of boundary read accesses due to insufficient boundary checks in probe_hfsplus.

The first issue occurs if the leaf count in a B-node is too small. The second happens while parsing a unicode description which is longer than 255 UTF-8 characters. The length is stored in a 16 bit integer, but the array in the struct is limited to 255 * 2, which is in sync with Apple's Open Source HFS+ implementation (HFSUniStr255).

Signed-off-by Tobias Stoeckmann <[email protected]>

---
You can trigger the issue with these two small proof of concepts, which contain byte sequences which imitate a HFS+ (but are broken beyond repair, as I was just fiddling around with bits until they did what I wanted to happen ;) ).

echo H4sICJKhyVgCA2lzby1wb2MtMQDt1LkNgDAURMFviSt1B9SCHNCAJTo\
hpXPjEkiBmfhFG+xWai1d684uIlJrOccVMU7zEimAj9qPp+Xw45VWLwgAAAA\
AAMAr3DUKOtIAIAAA | base64 -d - | gunzip > f1

echo H4sICKelyVgCA2lzby1wb2MtMgDt2rsJgFAMBdA88Ne6gbOIhQsIbuL\
48QkOYKue09wUKcOtMi/btlRZHVVElMxxrDm0XT80AXzVuj/d/HMTTMWlAAA\
AAAAA8CYZmfdYrl+AGfi8Exgn/EMAJAAA | base64 -d - | gunzip > f2

You can trigger the issue with the tool blkid this way:

# losetup -f $file
# blkid -p -n hfsplus /dev/loop0
---
 libblkid/src/superblocks/hfs.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
libblkid/src/superblocks/hfs.c +4 -2
index ee502cf61..ef746d4e2 100644
@@ -244,7 +244,8 @@ static int probe_hfsplus(blkid_probe pr, const struct blkid_idmag *mag)
 	leaf_node_head = be32_to_cpu(bnode->leaf_head);
 	leaf_node_size = be16_to_cpu(bnode->node_size);
 	leaf_node_count = be32_to_cpu(bnode->leaf_count);
-	if (leaf_node_count == 0)
+	if (leaf_node_size < sizeof(struct hfsplus_bnode_descriptor) +
+	    sizeof(struct hfsplus_catalog_key) || leaf_node_count == 0)
 		return 0;
 
 	leaf_block = (leaf_node_head * leaf_node_size) / blocksize;

@@ -284,7 +285,8 @@ static int probe_hfsplus(blkid_probe pr, const struct blkid_idmag *mag)
 	key = (struct hfsplus_catalog_key *)
 		&buf[sizeof(struct hfsplus_bnode_descriptor)];
 
-	if (be32_to_cpu(key->parent_id) != HFSPLUS_POR_CNID)
+	if (be32_to_cpu(key->parent_id) != HFSPLUS_POR_CNID ||
+	    be16_to_cpu(key->unicode_len > 255))
 		return 0;
 
 	blkid_probe_set_utf8label(pr, key->unicode,
Signature · 1 line
2.12.0
maintainer Applied
#1 lore raw
Quoted 2 lines · Tobias Stoeckmann
 libblkid/src/superblocks/hfs.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Applied, thanks.

Karel

Signature · 2 lines
 Karel Zak  <[email protected]>
 http://karelzak.blogspot.com
maintainer
#2 lore raw
In reply to Tobias Stoeckmann

libblkid/src/superblocks/hfs.c static int probe_hfsplus(blkid_probe pr, const struct blkid_idmag *mag) · line 289

Quoted 3 lines · Tobias Stoeckmann
libblkid/src/superblocks/hfs.c +2 -1
-	if (be32_to_cpu(key->parent_id) != HFSPLUS_POR_CNID)
+	if (be32_to_cpu(key->parent_id) != HFSPLUS_POR_CNID ||
+	    be16_to_cpu(key->unicode_len > 255))

Typo. Fixed. I guess it should be

    be16_to_cpu(key->unicode_len) > 255

Note than we have regression tests for almost all filesystems. So you

 $ cd tests
 $ ./run.sh blkid

to see result.

Karel

Signature · 2 lines
 Karel Zak  <[email protected]>
 http://karelzak.blogspot.com