Skip to content

Commit

Permalink
writer: Ensure re-setting xattrs are re-validated
Browse files Browse the repository at this point in the history
We did a lot of work to limit the size of xattrs, but
honggfuzz quickly figured out it was possible to bypass
this by specifying an xattr multiple times; the second
copy had special "replace existing value" code that
didn't check limits.

Fix this by always calling the "remove xattr" API when
setting (but ignoring the value). This ensures we
are consistently tracking limits.

Signed-off-by: Colin Walters <walters@verbum.org>
  • Loading branch information
cgwalters committed Sep 12, 2024
1 parent 3478877 commit 2ef9e9b
Showing 1 changed file with 2 additions and 15 deletions.
17 changes: 2 additions & 15 deletions libcomposefs/lcfs-writer.c
Original file line number Diff line number Diff line change
Expand Up @@ -1676,27 +1676,14 @@ int lcfs_node_set_xattr_internal(struct lcfs_node_s *node, const char *name,
return -1;
}

ssize_t index = find_xattr(node, name);

if (value_len > UINT16_MAX) {
errno = EINVAL;
return -1;
}

if (index >= 0) {
/* Already set, replace */
struct lcfs_xattr_s *xattr = &node->xattrs[index];
v = memdup(value, value_len);
if (v == NULL) {
errno = ENOMEM;
return -1;
}
free(xattr->value);
xattr->value = v;
xattr->value_len = value_len;
// Remove any existing value
(void)lcfs_node_unset_xattr(node, name);

return 0;
}
// Double the xattr metadata size, subtracting 1 to account for worst case alignment.
size_t entry_size = (2 * LCFS_INODE_XATTRMETA_SIZE) - 1 + namelen + value_len;
// If this is the first xattr, add size for the header
Expand Down

0 comments on commit 2ef9e9b

Please sign in to comment.