Skip to content

user-record: fix segfault when processing matchHostname field - #40979

Merged
keszybz merged 7 commits into
systemd:mainfrom
mrc0mmand:userdbctl-typo-segfault
Mar 7, 2026
Merged

user-record: fix segfault when processing matchHostname field#40979
keszybz merged 7 commits into
systemd:mainfrom
mrc0mmand:userdbctl-typo-segfault

Conversation

@mrc0mmand

@mrc0mmand mrc0mmand commented Mar 6, 2026

Copy link
Copy Markdown
Member

Fix a typo which causes a segfault when processing a user record
with matchHostname when it's an array instead of a simple string:

$ echo '{"userName":"crashhostarray","perMachine":[{"matchHostname":["host1","host2"],"locked":false}]}' | userdbctl -F -
Segmentation fault         (core dumped)

$ coredumpctl info
...
       Message: Process 1172301 (userdbctl) of user 1000 dumped core.

                Module libz.so.1 from rpm zlib-ng-2.3.3-1.fc43.x86_64
                Module libcrypto.so.3 from rpm openssl-3.5.4-2.fc43.x86_64
                Stack trace of thread 1172301:
                #0  0x00007fded7b3a656 __strcmp_evex (libc.so.6 + 0x159656)
                #1  0x00007fded7e95397 per_machine_hostname_match (libsystemd-shared-260.so + 0x295397)
                #2  0x00007fded7e955b5 per_machine_match (libsystemd-shared-260.so + 0x2955b5)
                #3  0x00007fded7e957c6 dispatch_per_machine (libsystemd-shared-260.so + 0x2957c6)
                #4  0x00007fded7e96c97 user_record_load (libsystemd-shared-260.so + 0x296c97)
                #5  0x000000000040572d display_user (/home/fsumsal/repos/@systemd/systemd/build/userdbctl + 0x572d)
                #6  0x00007fded7ea9727 dispatch_verb (libsystemd-shared-260.so + 0x2a9727)
                #7  0x000000000041077c run (/home/fsumsal/repos/@systemd/systemd/build/userdbctl + 0x1077c)
                #8  0x00000000004107ce main (/home/fsumsal/repos/@systemd/systemd/build/userdbctl + 0x107ce)
                #9  0x00007fded79e45b5 __libc_start_call_main (libc.so.6 + 0x35b5)
                #10 0x00007fded79e4668 __libc_start_main@@GLIBC_2.34 (libc.so.6 + 0x3668)
                #11 0x00000000004038d5 _start (/home/fsumsal/repos/@systemd/systemd/build/userdbctl + 0x38d5)
                ELF object binary architecture: AMD x86-64

@github-actions github-actions Bot added util-lib tests userdb please-review PR is ready for (re-)review by a maintainer labels Mar 6, 2026
@bluca bluca added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed please-review PR is ready for (re-)review by a maintainer labels Mar 6, 2026
YHNdnzj
YHNdnzj approved these changes Mar 6, 2026
View reviewed changes
Comment thread src/shared/user-record.c Outdated
@mrc0mmand
mrc0mmand force-pushed the userdbctl-typo-segfault branch from 1e951e1 to ac191a3 Compare March 6, 2026 15:45
@YHNdnzj YHNdnzj changed the title shared: fix segfault when processing matchHostname field user-record: fix segfault when processing matchHostname field Mar 6, 2026
@mrc0mmand mrc0mmand removed the good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed label Mar 6, 2026
@mrc0mmand

mrc0mmand commented Mar 6, 2026

Copy link
Copy Markdown
Member Author

I found a couple more issues, hence dropping the green flag. I'll prepare the necessary fixes and tests and push them here, so I don't have to split it across multiple PRs.

mrc0mmand added 4 commits March 6, 2026 17:24
@mrc0mmand
shared: fix segfault when processing matchHostname field
1e2517b 
Fix a typo which causes a segfault when processing a user record
with matchHostname when it's an array instead of a simple string:

$ echo '{"userName":"crashhostarray","perMachine":[{"matchHostname":["host1","host2"],"locked":false}]}' | userdbctl -F -
Segmentation fault         (core dumped)

$ coredumpctl info
...
       Message: Process 1172301 (userdbctl) of user 1000 dumped core.

                Module libz.so.1 from rpm zlib-ng-2.3.3-1.fc43.x86_64
                Module libcrypto.so.3 from rpm openssl-3.5.4-2.fc43.x86_64
                Stack trace of thread 1172301:
                #0  0x00007fded7b3a656 __strcmp_evex (libc.so.6 + 0x159656)
                #1  0x00007fded7e95397 per_machine_hostname_match (libsystemd-shared-260.so + 0x295397)
                #2  0x00007fded7e955b5 per_machine_match (libsystemd-shared-260.so + 0x2955b5)
                #3  0x00007fded7e957c6 dispatch_per_machine (libsystemd-shared-260.so + 0x2957c6)
                #4  0x00007fded7e96c97 user_record_load (libsystemd-shared-260.so + 0x296c97)
                #5  0x000000000040572d display_user (/home/fsumsal/repos/@systemd/systemd/build/userdbctl + 0x572d)
                #6  0x00007fded7ea9727 dispatch_verb (libsystemd-shared-260.so + 0x2a9727)
                #7  0x000000000041077c run (/home/fsumsal/repos/@systemd/systemd/build/userdbctl + 0x1077c)
                #8  0x00000000004107ce main (/home/fsumsal/repos/@systemd/systemd/build/userdbctl + 0x107ce)
                #9  0x00007fded79e45b5 __libc_start_call_main (libc.so.6 + 0x35b5)
                #10 0x00007fded79e4668 __libc_start_main@@GLIBC_2.34 (libc.so.6 + 0x3668)
                #11 0x00000000004038d5 _start (/home/fsumsal/repos/@systemd/systemd/build/userdbctl + 0x38d5)
                ELF object binary architecture: AMD x86-64
@mrc0mmand
shared: don't exclude valid min/max values for cgroup weight fields
76ab786 
1 and 10000 are valid cgroup weight values, but the condition was
incorrectly excluding them:

$ echo '{"userName":"crashhostarray","cpuWeight":1}' | userdbctl -F -
<stdin>:1:42: JSON field 'cpuWeight' is not in valid range 1…10000.

$ echo '{"userName":"crashhostarray","cpuWeight":10000}' | userdbctl -F -
<stdin>:1:42: JSON field 'cpuWeight' is not in valid range 1…10000.
@mrc0mmand
shared: don't leak memory from array fields
3c7bd94 
The fido2_hmac_salt/fido2_hmac_credential/recovery_key fields kept
leaking memory as the array itself wasn't deallocated after deallocating
each of its elements data:

$ build-san/userdbctl -F fuzz-corpus-userdb/auth-fido2.json
...
=================================================================
==1292840==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 112 byte(s) in 1 object(s) allocated from:
    #0 0x7f56f00e5e4b in realloc.part.0 (/lib64/libasan.so.8+0xe5e4b) (BuildId: 25975f766867e9e604dc5a71a8befeaed3301942)
    #1 0x7f56ed869e42 in greedy_realloc ../src/basic/alloc-util.c:65
    #2 0x7f56ed7ff5e9 in dispatch_fido2_hmac_salt ../src/shared/user-record.c:836
    #3 0x7f56edd73cbc in sd_json_dispatch_full ../src/libsystemd/sd-json/sd-json.c:5204
    #4 0x7f56edd745fc in sd_json_dispatch ../src/libsystemd/sd-json/sd-json.c:5276
    #5 0x7f56ed80100b in dispatch_privileged ../src/shared/user-record.c:998
    #6 0x7f56edd73cbc in sd_json_dispatch_full ../src/libsystemd/sd-json/sd-json.c:5204
    #7 0x7f56edd745fc in sd_json_dispatch ../src/libsystemd/sd-json/sd-json.c:5276
    #8 0x7f56ed80622c in user_record_load ../src/shared/user-record.c:1697
    #9 0x000000408c15 in display_user ../src/userdb/userdbctl.c:447
    #10 0x7f56ed83cc9a in dispatch_verb ../src/shared/verbs.c:137
    #11 0x00000041df2b in run ../src/userdb/userdbctl.c:1908
    #12 0x00000041dfbe in main ../src/userdb/userdbctl.c:1911
    #13 0x7f56ec8105b4 in __libc_start_call_main (/lib64/libc.so.6+0x35b4) (BuildId: 2b5beec0fd24fe9c9f43eddfdd5facf0b8a1b805)
    #14 0x7f56ec810667 in __libc_start_main@@GLIBC_2.34 (/lib64/libc.so.6+0x3667) (BuildId: 2b5beec0fd24fe9c9f43eddfdd5facf0b8a1b805)
    #15 0x000000404a44 in _start (/home/fsumsal/repos/@systemd/systemd/build-san/userdbctl+0x404a44) (BuildId: 19e8b7e7b7038d2cea20bc18a55bea2a9e4406d5)

Direct leak of 64 byte(s) in 1 object(s) allocated from:
    #0 0x7f56f00e5e4b in realloc.part.0 (/lib64/libasan.so.8+0xe5e4b) (BuildId: 25975f766867e9e604dc5a71a8befeaed3301942)
    #1 0x7f56ed869e42 in greedy_realloc ../src/basic/alloc-util.c:65
    #2 0x7f56ed7fe779 in dispatch_fido2_hmac_credential_array ../src/shared/user-record.c:775
    #3 0x7f56edd73cbc in sd_json_dispatch_full ../src/libsystemd/sd-json/sd-json.c:5204
    #4 0x7f56edd745fc in sd_json_dispatch ../src/libsystemd/sd-json/sd-json.c:5276
    #5 0x7f56ed80622c in user_record_load ../src/shared/user-record.c:1697
    #6 0x000000408c15 in display_user ../src/userdb/userdbctl.c:447
    #7 0x7f56ed83cc9a in dispatch_verb ../src/shared/verbs.c:137
    #8 0x00000041df2b in run ../src/userdb/userdbctl.c:1908
    #9 0x00000041dfbe in main ../src/userdb/userdbctl.c:1911
    #10 0x7f56ec8105b4 in __libc_start_call_main (/lib64/libc.so.6+0x35b4) (BuildId: 2b5beec0fd24fe9c9f43eddfdd5facf0b8a1b805)
    #11 0x7f56ec810667 in __libc_start_main@@GLIBC_2.34 (/lib64/libc.so.6+0x3667) (BuildId: 2b5beec0fd24fe9c9f43eddfdd5facf0b8a1b805)
    #12 0x000000404a44 in _start (/home/fsumsal/repos/@systemd/systemd/build-san/userdbctl+0x404a44) (BuildId: 19e8b7e7b7038d2cea20bc18a55bea2a9e4406d5)

SUMMARY: AddressSanitizer: 176 byte(s) leaked in 2 allocation(s).
@mrc0mmand
man: fix short option for userdbctl's --from-file=
ad1c7df
@mrc0mmand
mrc0mmand force-pushed the userdbctl-typo-segfault branch from ac191a3 to 9b0773d Compare March 6, 2026 17:39
@mrc0mmand

mrc0mmand commented Mar 6, 2026

Copy link
Copy Markdown
Member Author

I added a simple fuzzer with the corpus I originally found (most of) these issues with. It should serve both as a regression test for these issues and something we could, potentially, find more issues with in oss-fuzz.

@bluca hopefully the corpus isn't too big. I thought about minimizing the JSON files, but I'm not sure if sacrificing readability is worth it in this case (on the other hand, I'm not sure if anyone is going to read these files anyway).

bluca
bluca approved these changes Mar 6, 2026
View reviewed changes

@bluca bluca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's text, so it compresses well, so should be fine

@github-actions github-actions Bot added documentation build-system meson please-review PR is ready for (re-)review by a maintainer labels Mar 6, 2026
@mrc0mmand
fuzz: add a fuzzer for user records
be0db50 
Add a simple fuzzer that verifies our machinery for parsing user records
from JSON works as intended.

The initial corpus was created with the help of Claude, so we have a
bunch of valid user records with as much fields as possible for the
initial corpus.
@mrc0mmand
mrc0mmand force-pushed the userdbctl-typo-segfault branch from 9b0773d to be0db50 Compare March 6, 2026 18:03
@mrc0mmand

mrc0mmand commented Mar 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Pushed a fix for another crash found during fuzzing (last one, for this PR, I promise).

YHNdnzj
YHNdnzj reviewed Mar 6, 2026
View reviewed changes
Comment thread src/shared/user-record.c Outdated
@mrc0mmand
mrc0mmand force-pushed the userdbctl-typo-segfault branch from c789e23 to a7f1670 Compare March 6, 2026 21:51
@YHNdnzj YHNdnzj added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed please-review PR is ready for (re-)review by a maintainer labels Mar 6, 2026
keszybz
keszybz approved these changes Mar 7, 2026
View reviewed changes

@keszybz keszybz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@keszybz keszybz added ci-failure-appears-unrelated and removed good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed labels Mar 7, 2026
@keszybz
keszybz merged commit 0bec119 into systemd:main Mar 7, 2026
56 of 59 checks passed
@mrc0mmand
mrc0mmand deleted the userdbctl-typo-segfault branch March 7, 2026 17:07
@keszybz

keszybz commented Mar 11, 2026

Copy link
Copy Markdown
Member

Backport queued for v259.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@YHNdnzj YHNdnzj YHNdnzj approved these changes
@keszybz keszybz keszybz approved these changes
@bluca bluca bluca approved these changes

Assignees

No one assigned

Labels

build-system ci-failure-appears-unrelated documentation meson tests userdb util-lib

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mrc0mmand @keszybz @bluca @YHNdnzj @yuwata