| Message ID | 20250109121530.21728-1-cgoettsche@seltendoof.de (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Petr Lautrbach |
| Headers | show |
| Series | libselinux: limit node depth while parsing compiled fcontexts | expand |
On Thu, Jan 9, 2025 at 7:15 AM Christian Göttsche <cgoettsche@seltendoof.de> wrote: > > From: Christian Göttsche <cgzones@googlemail.com> > > Limit the node depth while parsing a pre-compiled fcontext definition to > avoid unlimited recursions causing stack overflows. > > Use a sufficiently high value of 32, instead of the node depth of > currently 3 for generating a database, to not unnecessarily limit > custom changes. > > Fixes: 92306daf ("libselinux: rework selabel_file(5) database") > Reported-by: oss-fuzz (issues 388615595 and 388592303) > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > --- > libselinux/src/label_file.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c > index 56e20949..125eb601 100644 > --- a/libselinux/src/label_file.c > +++ b/libselinux/src/label_file.c > @@ -674,12 +674,16 @@ static int load_mmap_regex_spec(struct mmap_area *mmap_area, bool validating, bo > } > > static int load_mmap_spec_node(struct mmap_area *mmap_area, const char *path, bool validating, bool do_load_precompregex, > - struct spec_node *node, bool is_root, uint8_t inputno, const struct context_array *ctx_array) > + struct spec_node *node, const unsigned depth, uint8_t inputno, const struct context_array *ctx_array) > { > uint32_t data_u32, lspec_num, rspec_num, children_num; > uint16_t data_u16, stem_len; > + const bool is_root = (depth == 0); > int rc; > > + if (depth >= 32) I would like to see a comment here explaining that this check is to prevent unlimited recursions and that 32 was arbitrarily chosen as a sufficiently high value that is not expected to occur. (Or something to that effect.) Jim > + return -1; > + > node->from_mmap = true; > > > @@ -794,7 +798,7 @@ static int load_mmap_spec_node(struct mmap_area *mmap_area, const char *path, bo > node->children_alloc = children_num; > > for (uint32_t i = 0; i < children_num; i++) { > - rc = load_mmap_spec_node(mmap_area, path, validating, do_load_precompregex, &node->children[i], false, inputno, ctx_array); > + rc = load_mmap_spec_node(mmap_area, path, validating, do_load_precompregex, &node->children[i], depth + 1, inputno, ctx_array); > if (rc) > return -1; > > @@ -969,7 +973,7 @@ end_arch_check: > > rc = load_mmap_spec_node(mmap_area, path, rec->validating, > reg_version_matches && reg_arch_matches, > - root, true, > + root, 0, > inputno, > &ctx_array); > if (rc) > -- > 2.47.1 > >
diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c index 56e20949..125eb601 100644 --- a/libselinux/src/label_file.c +++ b/libselinux/src/label_file.c @@ -674,12 +674,16 @@ static int load_mmap_regex_spec(struct mmap_area *mmap_area, bool validating, bo } static int load_mmap_spec_node(struct mmap_area *mmap_area, const char *path, bool validating, bool do_load_precompregex, - struct spec_node *node, bool is_root, uint8_t inputno, const struct context_array *ctx_array) + struct spec_node *node, const unsigned depth, uint8_t inputno, const struct context_array *ctx_array) { uint32_t data_u32, lspec_num, rspec_num, children_num; uint16_t data_u16, stem_len; + const bool is_root = (depth == 0); int rc; + if (depth >= 32) + return -1; + node->from_mmap = true; @@ -794,7 +798,7 @@ static int load_mmap_spec_node(struct mmap_area *mmap_area, const char *path, bo node->children_alloc = children_num; for (uint32_t i = 0; i < children_num; i++) { - rc = load_mmap_spec_node(mmap_area, path, validating, do_load_precompregex, &node->children[i], false, inputno, ctx_array); + rc = load_mmap_spec_node(mmap_area, path, validating, do_load_precompregex, &node->children[i], depth + 1, inputno, ctx_array); if (rc) return -1; @@ -969,7 +973,7 @@ end_arch_check: rc = load_mmap_spec_node(mmap_area, path, rec->validating, reg_version_matches && reg_arch_matches, - root, true, + root, 0, inputno, &ctx_array); if (rc)