Add support for setxattr and listxattr system calls#986
Add support for setxattr and listxattr system calls#986celinehoang177 wants to merge 2 commits intomainfrom
Conversation
ac6973f to
4f27bcf
Compare
End-to-End Test ReportTest PreviewUnified Test Report grate harness
Cases
wasm harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
Test Results by Category
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
vidyalakshmir
left a comment
There was a problem hiding this comment.
@celinehoang177 What about the changes to glibc?
4f27bcf to
8e360b2
Compare
End-to-End Test ReportTest PreviewUnified Test Report grate harness
Cases
wasm harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
Test Results by Category
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Yaxuan-w
left a comment
There was a problem hiding this comment.
LGTM. Is there any reason that we add new files for those two syscalls instead of just changing the source in glibc?
rennergade
left a comment
There was a problem hiding this comment.
Pointed out a few issues, also need to add a test.
|
@celinehoang177 Please address the comments above. |
8e360b2 to
6f1286a
Compare
End-to-End Test ReportTest PreviewUnified Test Report grate harness
Cases
wasm harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
Test Results by Category
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ee2c8ec to
b74524d
Compare
I fixed it. It's now just changing the source in glibc. |
End-to-End Test ReportTest PreviewUnified Test Report grate harness
Cases
wasm harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
Test Results by Category
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||
| sc_convert_buf(list_arg, list_cageid, cageid) as *mut libc::c_char | ||
| }; | ||
| let size = sc_convert_sysarg_to_usize(size_arg, size_cageid, cageid); |
There was a problem hiding this comment.
In the earlier line, guest memory pointers are translated using sc_convert_buf without validating the guest's provided size argument. This allows a malicious guest to allocate a tiny buffer but pass a massive size, tricking the host kernel into writing outside the guest's memory boundaries (a buffer overflow).
Proposed Fix: We need to add a bounds check to validate that the entire contiguous memory region from pointer to pointer + size resides completely within the allowed cage memory. (pointer is list_arg).
@Yaxuan-w @rennergade @qianxichen233
| } else { | ||
| sc_convert_buf(value_arg, value_cageid, cageid) as *const libc::c_void | ||
| }; | ||
| let size = sc_convert_sysarg_to_usize(size_arg, size_cageid, cageid); |
There was a problem hiding this comment.
Similar to the comment for listxattr, there can be an out-of-bounds read here. We need to check if pointer + size is within the allowed cage memory.
If the guest allocates a 5-byte buffer containing "hello", but calls setxattr with a size of 1000, the host kernel will read 1000 bytes starting from that pointer and save it to the file's extended attributes. The guest just forced the host kernel to read 995 bytes of adjacent memory and save it to the disk. The guest can then read that file's extended attributes to steal the host's memory.
| if ret < 0 { | ||
| let errno = get_errno(); | ||
| return handle_errno(errno, "listxattr"); | ||
| } |
There was a problem hiding this comment.
The setxattr and listxattr implementations allow the guest to operate on arbitrary xattr namespaces. Since the host kernel evaluates these calls using the sandbox runtime's privileges, a malicious guest can write to security.* attributes (e.g. security.capability to grant root capabilities) or modify ACLs via system.* — fully bypassing host access controls. All xattr operations must be restricted to the user.* namespace, with anything else rejected at the shim layer.
@Yaxuan-w @rennergade @qianxichen233
There was a problem hiding this comment.
Approving this. The changes requested would be tackled in a different issue and this issue. Please fix the merged conflicts.
|
Needs to resolve merge conflicts before merging |
83e55e2 to
af8d239
Compare
af8d239 to
105ca48
Compare
End-to-End Test ReportTest PreviewUnified Test Report grate harness
Cases
static harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
wasm harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
Test Results by Category
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Implements support for setxattr() and listxattr() system calls in Lind-Wasm, enabling compatibility with Python 3.14.3 and other applications that use extended file attributes.
Changes:
Closes #889