Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

segfault installing packages (in very strange edge cases) #1833

Closed
achilleas-k opened this issue Nov 16, 2021 · 9 comments · Fixed by #1844
Closed

segfault installing packages (in very strange edge cases) #1833

achilleas-k opened this issue Nov 16, 2021 · 9 comments · Fixed by #1844
Assignees
Labels

Comments

@achilleas-k
Copy link

Hi all!

I'm getting segfaults from rpm when installing packages in osbuild. The set of circumstances that cause the segfault are very strange and very sensitive to changes, so I might be giving a lot of useless information here.

The issue happens with rpm 4.16.1.3 on CentOS Stream 9. I haven't managed to reproduce it on other distros or distro versions, even with the same packages, though some package versions might differ. It also seems to be sensitive to the following:

  • Order of packages to be installed
  • Specific package set to be installed: adding or removing packages can change whether the segfault happens or not
  • Duplicating packages: having duplicates in the package list seems to cause it, while deduplicating the list fixes it
  • Excluding docs

I managed to get a backtrace, though I haven't managed to reproduce it in an interactive environment so I haven't managed to look around. Here's the bt:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7f7b226 in hex2bin (h=h@entry=0x555555625dc0, tag=tag@entry=5090, num=num@entry=801, len=224) at /usr/src/debug/rpm-4.16.1.3-7.el9.x86_64/lib/rpmfi.c:1505
1505                    *t = (rnibble(s[0]) << 4) | rnibble(s[1]);
#0  0x00007ffff7f7b226 in hex2bin (h=h@entry=0x555555625dc0, tag=tag@entry=5090, num=num@entry=801, len=224) at /usr/src/debug/rpm-4.16.1.3-7.el9.x86_64/lib/rpmfi.c:1505
#1  0x00007ffff7f7bc2f in rpmfilesPopulate (flags=65538, h=0x555555625dc0, fi=0x555555633e80) at /usr/src/debug/rpm-4.16.1.3-7.el9.x86_64/lib/rpmfi.c:1580
#2  rpmfilesNew (pool=0x5555555c8db0, h=0x555555625dc0, tagN=<optimized out>, flags=65538) at /usr/src/debug/rpm-4.16.1.3-7.el9.x86_64/lib/rpmfi.c:1642
#3  0x00007ffff7f834d1 in addTE (p=0x555555625c30, h=0x555555625dc0, key=0x55555561ef50, relocs=<optimized out>) at /usr/src/debug/rpm-4.16.1.3-7.el9.x86_64/lib/rpmte.c:188
#4  0x00007ffff7f66da6 in rpmteNew (addop=0, relocs=0x0, key=0x55555561ef50, type=TR_ADDED, h=0x555555625dc0, ts=0x55555555fb40) at /usr/src/debug/rpm-4.16.1.3-7.el9.x86_64/lib/rpmte.c:258
#5  addPackage (ts=0x55555555fb40, h=0x555555625dc0, key=0x55555561ef50, op=<optimized out>, relocs=0x0) at /usr/src/debug/rpm-4.16.1.3-7.el9.x86_64/lib/depends.c:422
#6  0x00007ffff7f8b567 in rpmInstall (ts=<optimized out>, ia=<optimized out>, fileArgv=<optimized out>) at /usr/src/debug/rpm-4.16.1.3-7.el9.x86_64/lib/rpminstall.c:599
#7  0x0000555555556b9c in main (argc=8, argv=<optimized out>) at /usr/src/debug/rpm-4.16.1.3-7.el9.x86_64/rpm.c:265

If you'd like to reproduce it, install osbuild (version 40 or newer, for CS9 support) and run the attached manifest as root:

osbuild --checkpoint build rpm-segfault.txt

The manifest is in JSON but I renamed it to TXT to attach it to the issue.
The --checkpoint build flag will save a checkpoint of the first part of the build so repeated runs will be faster. By default these are saved in .osbuild in the current working directory, but you can change that with the --store option.

I've been trying to pinpoint the cause for a couple of days now and it's making less sense the more I look at it.

Let me know if there's any more information you need or if I can help in any way.

rpm-segfault.txt

@mlschroe
Copy link
Contributor

What's very suspicious is the "len=224" argument. It's supposed to be the digest length in bytes.

@pmatilai
Copy link
Member

That'd be IMA signatures, which are a new thing in CS9. Probably this issue:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=2018937

@pmatilai pmatilai added the bug label Nov 17, 2021
@mlschroe
Copy link
Contributor

The code in rpmsignfiles.c should at least check if the signature sizes match and not just use the size of the last signature...

Anyway, the sizes are implicit through the string array, so we can simply ignore the RPMTAG_FILESIGNATURELENGTH from the header and do the right thing in the implementation.

We can make fi->signaturelen an array, add fi->maxsignaturelen and add a new hex2bin variant that first calculates the max signature size, puts in in fi->maxsignaturelen and then converts the individual entries populating fi->signaturelen.

@mlschroe
Copy link
Contributor

mlschroe commented Nov 17, 2021

Basically like the veritysigs implementation... I didn't know about those.

@pmatilai
Copy link
Member

Yup. Keeping the hex-encoded strings as such in memory (as done in the rough patch in bugzilla) is would be a terrible waste of memory.

@pmatilai
Copy link
Member

...on a related note, we should only bother with keeping this stuff in memory if IMA plugin is loaded, it's a non-trivial amount of memory even in binary form for which we have absolutely zero use if they don't get used. Ditto with fsverity.

@DemiMarie
Copy link
Contributor

Is this an exploitable vulnerability? This also shows why IMA and fsverity signatures belong in the main header, where they cannot be tampered with.

@achilleas-k
Copy link
Author

achilleas-k commented Nov 23, 2021

What's the situation with this? Is there anything I can do to work around it consistently?

Never mind, I see there's an update on the RHBZ.

@pmatilai
Copy link
Member

To work around, one of the following should do:

  • rpm -e rpm-plugin-ima
  • echo "%__transaction_ima %{nil}" > /etc/rpm/macros.ima

pmatilai added a commit to pmatilai/rpm that referenced this issue Nov 26, 2021
, RhBug:2018937)

ECDSA signatures can vary in length, but the IMA code assumes constant
lengths and thus may either place invalid signatures on disk from
either truncating or overshooting, and segfault if the stars are just
so.

Luckily the signatures are stored as strings so we can calculate the
actual lengths at runtime and ignore the stored constant length info.
Extend hex2bin() to optionally calculate the lengths and maximum,
and use these for returning IMA data from the rpmfi(les) API.

Additionally update the signing code to store the largest IMA signature
length rather than what happened to be last to be on the safe side.
We can't rely on this value due to invalid packages being out there,
but then we need to calculate the lengths on rpmfiles populate so there's
not a lot to gain anyhow.

Fixes: rpm-software-management#1833
pmatilai added a commit to pmatilai/rpm that referenced this issue Nov 29, 2021
, RhBug:2018937)

ECDSA signatures can vary in length, but the IMA code assumes constant
lengths and thus may either place invalid signatures on disk from
either truncating or overshooting, and segfault if the stars are just
so.

Luckily the signatures are stored as strings so we can calculate the
actual lengths at runtime and ignore the stored constant length info.
Extend hex2bin() to optionally calculate the lengths and maximum,
and use these for returning IMA data from the rpmfi(les) API.

Additionally update the signing code to store the largest IMA signature
length rather than what happened to be last to be on the safe side.
We can't rely on this value due to invalid packages being out there,
but then we need to calculate the lengths on rpmfiles populate so there's
not a lot to gain anyhow.

Fixes: rpm-software-management#1833
pmatilai added a commit to pmatilai/rpm that referenced this issue Nov 29, 2021
, RhBug:2018937)

At least ECDSA and RSA signatures can vary in length, but the IMA code
assumes constant lengths and thus may either place invalid signatures on
disk from either truncating or overshooting, and segfault if the stars are
just so.

Luckily the signatures are stored as strings so we can calculate the
actual lengths at runtime and ignore the stored constant length info.
Extend hex2bin() to optionally calculate the lengths and maximum,
and use these for returning IMA data from the rpmfi(les) API.

Additionally update the signing code to store the largest IMA signature
length rather than what happened to be last to be on the safe side.
We can't rely on this value due to invalid packages being out there,
but then we need to calculate the lengths on rpmfiles populate so there's
not a lot to gain anyhow.

Fixes: rpm-software-management#1833
@pmatilai pmatilai self-assigned this Dec 2, 2021
pmatilai added a commit to pmatilai/rpm that referenced this issue Dec 2, 2021
, RhBug:2018937)

At least ECDSA and RSA signatures can vary in length, but the IMA code
assumes constant lengths and thus may either place invalid signatures on
disk from either truncating or overshooting, and segfault if the stars are
just so.

Luckily the signatures are stored as strings so we can calculate the
actual lengths at runtime and ignore the stored constant length info.
Extend hex2bin() to optionally calculate the lengths and maximum,
and use these for returning IMA data from the rpmfi(les) API.

Additionally update the signing code to store the largest IMA signature
length rather than what happened to be last to be on the safe side.
We can't rely on this value due to invalid packages being out there,
but then we need to calculate the lengths on rpmfiles populate so there's
not a lot to gain anyhow.

Fixes: rpm-software-management#1833
dmnks pushed a commit to dmnks/rpm that referenced this issue Dec 6, 2021
, RhBug:2018937)

At least ECDSA and RSA signatures can vary in length, but the IMA code
assumes constant lengths and thus may either place invalid signatures on
disk from either truncating or overshooting, and segfault if the stars are
just so.

Luckily the signatures are stored as strings so we can calculate the
actual lengths at runtime and ignore the stored constant length info.
Extend hex2bin() to optionally calculate the lengths and maximum,
and use these for returning IMA data from the rpmfi(les) API.

Additionally update the signing code to store the largest IMA signature
length rather than what happened to be last to be on the safe side.
We can't rely on this value due to invalid packages being out there,
but then we need to calculate the lengths on rpmfiles populate so there's
not a lot to gain anyhow.

Fixes: rpm-software-management#1833
dmnks pushed a commit to dmnks/rpm that referenced this issue Dec 6, 2021
, RhBug:2018937)

At least ECDSA and RSA signatures can vary in length, but the IMA code
assumes constant lengths and thus may either place invalid signatures on
disk from either truncating or overshooting, and segfault if the stars are
just so.

Luckily the signatures are stored as strings so we can calculate the
actual lengths at runtime and ignore the stored constant length info.
Extend hex2bin() to optionally calculate the lengths and maximum,
and use these for returning IMA data from the rpmfi(les) API.

Additionally update the signing code to store the largest IMA signature
length rather than what happened to be last to be on the safe side.
We can't rely on this value due to invalid packages being out there,
but then we need to calculate the lengths on rpmfiles populate so there's
not a lot to gain anyhow.

Fixes: rpm-software-management#1833
pmatilai added a commit to pmatilai/rpm that referenced this issue Dec 13, 2021
, RhBug:2018937)

At least ECDSA and RSA signatures can vary in length, but the IMA code
assumes constant lengths and thus may either place invalid signatures on
disk from either truncating or overshooting, and segfault if the stars are
just so.

Luckily the signatures are stored as strings so we can calculate the
actual lengths at runtime and ignore the stored constant length info.
Extend hex2bin() to optionally calculate the lengths and maximum,
and use these for returning IMA data from the rpmfi(les) API.

Additionally update the signing code to store the largest IMA signature
length rather than what happened to be last to be on the safe side.
We can't rely on this value due to invalid packages being out there,
but then we need to calculate the lengths on rpmfiles populate so there's
not a lot to gain anyhow.

Fixes: rpm-software-management#1833
pmatilai added a commit that referenced this issue Dec 13, 2021
At least ECDSA and RSA signatures can vary in length, but the IMA code
assumes constant lengths and thus may either place invalid signatures on
disk from either truncating or overshooting, and segfault if the stars are
just so.

Luckily the signatures are stored as strings so we can calculate the
actual lengths at runtime and ignore the stored constant length info.
Extend hex2bin() to optionally calculate the lengths and maximum,
and use these for returning IMA data from the rpmfi(les) API.

Additionally update the signing code to store the largest IMA signature
length rather than what happened to be last to be on the safe side.
We can't rely on this value due to invalid packages being out there,
but then we need to calculate the lengths on rpmfiles populate so there's
not a lot to gain anyhow.

Fixes: #1833
pmatilai added a commit that referenced this issue Dec 13, 2021
Commit a79d7ae had two major flaws:

Firstly, it negletted to update the part that copies the data in place to
match the new layout where data lengths are not always equal, and the for
loop would overshoot its bounds on data shorter than maximum.

Secondly, rpmfilesFSignature() would now crash on packages with no IMA
signatures because fi->signaturelengths is not allocated. Take care not
to change API behavior wrt *len return value: set to zero if no
signatures are present.
pmatilai added a commit to pmatilai/rpm that referenced this issue Feb 7, 2022
…g:2018937)

At least ECDSA and RSA signatures can vary in length, but the IMA code
assumes constant lengths and thus may either place invalid signatures on
disk from either truncating or overshooting, and segfault if the stars are
just so.

As we can't assume static lengths and attempts to use maximum length
have proven problematic for other reasons, use a data structure that
can actually handle variable length data properly: store offsets into
the decoded binary blob and use them to calculate lengths when needed,
empty data is handled with special zero-block at zero offset whose size
is known beforehand. This avoids a whole class of silly overflow issues
with multiplying, makes zero-length data actually presentable in the
data structure and saves memory too.

Additionally update the signing code to store the largest IMA signature
length rather than what happened to be last to be on the safe side.
We can't rely on this value due to invalid packages being out there,
but then we need to calculate the lengths on rpmfiles populate so there's
not a lot to gain anyhow.

Fixes: rpm-software-management#1833
pmatilai added a commit to pmatilai/rpm that referenced this issue Feb 7, 2022
…g:2018937)

At least ECDSA and RSA signatures can vary in length, but the IMA code
assumes constant lengths and thus may either place invalid signatures on
disk from either truncating or overshooting, and segfault if the stars are
just so.

As we can't assume static lengths and attempts to use maximum length
have proven problematic for other reasons, use a data structure that
can actually handle variable length data properly: store offsets into
the decoded binary blob and use them to calculate lengths when needed,
empty data is handled with special zero-block at zero offset whose size
is known beforehand. This avoids a whole class of silly overflow issues
with multiplying, makes zero-length data actually presentable in the
data structure and saves memory too.

Additionally update the signing code to store the largest IMA signature
length rather than what happened to be last to be on the safe side.
We can't rely on this value due to invalid packages being out there,
but then we need to calculate the lengths on rpmfiles populate so there's
not a lot to gain anyhow.

Fixes: rpm-software-management#1833
pmatilai added a commit to pmatilai/rpm that referenced this issue Feb 7, 2022
…g:2018937)

At least ECDSA and RSA signatures can vary in length, but the IMA code
assumes constant lengths and thus may either place invalid signatures on
disk from either truncating or overshooting, and segfault if the stars are
just so.

As we can't assume static lengths and attempts to use maximum length
have proven problematic for other reasons, use a data structure that
can actually handle variable length data properly: store offsets into
the decoded binary blob and use them to calculate lengths when needed,
empty data is handled with special zero-block at zero offset whose size
is known beforehand. This avoids a whole class of silly overflow issues
with multiplying, makes zero-length data actually presentable in the
data structure and saves memory too.

Additionally update the signing code to store the largest IMA signature
length rather than what happened to be last to be on the safe side.
We can't rely on this value due to invalid packages being out there,
but then we need to calculate the lengths on rpmfiles populate so there's
not a lot to gain anyhow.

Fixes: rpm-software-management#1833
pmatilai added a commit to pmatilai/rpm that referenced this issue Feb 7, 2022
…g:2018937)

At least ECDSA and RSA signatures can vary in length, but the IMA code
assumes constant lengths and thus may either place invalid signatures on
disk from either truncating or overshooting, and segfault if the stars are
just so.

As we can't assume static lengths and attempts to use maximum length
have proven problematic for other reasons, use a data structure that
can actually handle variable length data properly: store offsets into
the decoded binary blob and use them to calculate lengths when needed,
empty data is handled with special zero-block at zero offset whose size
is known beforehand. This avoids a whole class of silly overflow issues
with multiplying, makes zero-length data actually presentable in the
data structure and saves memory too.

Additionally update the signing code to store the largest IMA signature
length rather than what happened to be last to be on the safe side.
We can't rely on this value due to invalid packages being out there,
but then we need to calculate the lengths on rpmfiles populate so there's
not a lot to gain anyhow.

Fixes: rpm-software-management#1833
pmatilai added a commit to pmatilai/rpm that referenced this issue Feb 8, 2022
pmatilai added a commit to pmatilai/rpm that referenced this issue Feb 8, 2022
…g:2018937)

At least ECDSA and RSA signatures can vary in length, but the IMA code
assumes constant lengths and thus may either place invalid signatures on
disk from either truncating or overshooting, and segfault if the stars are
just so.

As we can't assume static lengths and attempts to use maximum length
have proven problematic for other reasons, use a data structure that
can actually handle variable length data properly: store offsets into
the decoded binary blob and use them to calculate lengths when needed,
empty data is simply consequtive identical offsets. This avoids a whole
class of silly overflow issues with multiplying, makes zero-length data
actually presentable in the data structure and saves memory too.

Add tests to show behavior with variable length signatures and missing
signatures.

Additionally update the signing code to store the largest IMA signature
length rather than what happened to be last to be on the safe side.
We can't rely on this value due to invalid packages being out there,
but then we need to calculate the lengths on rpmfiles populate so there's
not a lot to gain anyhow.

Fixes: rpm-software-management#1833
pmatilai added a commit to pmatilai/rpm that referenced this issue Feb 9, 2022
…g:2018937)

At least ECDSA and RSA signatures can vary in length, but the IMA code
assumes constant lengths and thus may either place invalid signatures on
disk from either truncating or overshooting, and segfault if the stars are
just so.

As we can't assume static lengths and attempts to use maximum length
have proven problematic for other reasons, use a data structure that
can actually handle variable length data properly: store offsets into
the decoded binary blob and use them to calculate lengths when needed,
empty data is simply consequtive identical offsets. This avoids a whole
class of silly overflow issues with multiplying, makes zero-length data
actually presentable in the data structure and saves memory too.

Add tests to show behavior with variable length signatures and missing
signatures.

Additionally update the signing code to store the largest IMA signature
length rather than what happened to be last to be on the safe side.
We can't rely on this value due to invalid packages being out there,
but then we need to calculate the lengths on rpmfiles populate so there's
not a lot to gain anyhow.

Fixes: rpm-software-management#1833
pmatilai added a commit to pmatilai/rpm that referenced this issue Feb 9, 2022
…g:2018937)

At least ECDSA and RSA signatures can vary in length, but the IMA code
assumes constant lengths and thus may either place invalid signatures on
disk from either truncating or overshooting, and segfault if the stars are
just so.

As we can't assume static lengths and attempts to use maximum length
have proven problematic for other reasons, use a data structure that
can actually handle variable length data properly: store offsets into
the decoded binary blob and use them to calculate lengths when needed,
empty data is simply consequtive identical offsets. This avoids a whole
class of silly overflow issues with multiplying, makes zero-length data
actually presentable in the data structure and saves memory too.

Add tests to show behavior with variable length signatures and missing
signatures.

Additionally update the signing code to store the largest IMA signature
length rather than what happened to be last to be on the safe side.
We can't rely on this value due to invalid packages being out there,
but then we need to calculate the lengths on rpmfiles populate so there's
not a lot to gain anyhow.

Fixes: rpm-software-management#1833
pmatilai added a commit to pmatilai/rpm that referenced this issue Feb 10, 2022
…g:2018937)

At least ECDSA and RSA signatures can vary in length, but the IMA code
assumes constant lengths and thus may either place invalid signatures on
disk from either truncating or overshooting, and segfault if the stars are
just so.

As we can't assume static lengths and attempts to use maximum length
have proven problematic for other reasons, use a data structure that
can actually handle variable length data properly: store offsets into
the decoded binary blob and use them to calculate lengths when needed,
empty data is simply consequtive identical offsets. This avoids a whole
class of silly overflow issues with multiplying, makes zero-length data
actually presentable in the data structure and saves memory too.

Add tests to show behavior with variable length signatures and missing
signatures.

Additionally update the signing code to store the largest IMA signature
length rather than what happened to be last to be on the safe side.
We can't rely on this value due to invalid packages being out there,
but then we need to calculate the lengths on rpmfiles populate so there's
not a lot to gain anyhow.

Fixes: rpm-software-management#1833
pmatilai added a commit that referenced this issue Feb 11, 2022
At least ECDSA and RSA signatures can vary in length, but the IMA code
assumes constant lengths and thus may either place invalid signatures on
disk from either truncating or overshooting, and segfault if the stars are
just so.

As we can't assume static lengths and attempts to use maximum length
have proven problematic for other reasons, use a data structure that
can actually handle variable length data properly: store offsets into
the decoded binary blob and use them to calculate lengths when needed,
empty data is simply consequtive identical offsets. This avoids a whole
class of silly overflow issues with multiplying, makes zero-length data
actually presentable in the data structure and saves memory too.

Add tests to show behavior with variable length signatures and missing
signatures.

Additionally update the signing code to store the largest IMA signature
length rather than what happened to be last to be on the safe side.
We can't rely on this value due to invalid packages being out there,
but then we need to calculate the lengths on rpmfiles populate so there's
not a lot to gain anyhow.

Fixes: #1833
dmnks pushed a commit to dmnks/rpm that referenced this issue Jun 7, 2022
…g:2018937)

At least ECDSA and RSA signatures can vary in length, but the IMA code
assumes constant lengths and thus may either place invalid signatures on
disk from either truncating or overshooting, and segfault if the stars are
just so.

As we can't assume static lengths and attempts to use maximum length
have proven problematic for other reasons, use a data structure that
can actually handle variable length data properly: store offsets into
the decoded binary blob and use them to calculate lengths when needed,
empty data is simply consequtive identical offsets. This avoids a whole
class of silly overflow issues with multiplying, makes zero-length data
actually presentable in the data structure and saves memory too.

Add tests to show behavior with variable length signatures and missing
signatures.

Additionally update the signing code to store the largest IMA signature
length rather than what happened to be last to be on the safe side.
We can't rely on this value due to invalid packages being out there,
but then we need to calculate the lengths on rpmfiles populate so there's
not a lot to gain anyhow.

Fixes: rpm-software-management#1833
(cherry picked from commit 07f1d31)
dmnks pushed a commit to dmnks/rpm that referenced this issue Jun 7, 2022
…g:2018937)

At least ECDSA and RSA signatures can vary in length, but the IMA code
assumes constant lengths and thus may either place invalid signatures on
disk from either truncating or overshooting, and segfault if the stars are
just so.

As we can't assume static lengths and attempts to use maximum length
have proven problematic for other reasons, use a data structure that
can actually handle variable length data properly: store offsets into
the decoded binary blob and use them to calculate lengths when needed,
empty data is simply consequtive identical offsets. This avoids a whole
class of silly overflow issues with multiplying, makes zero-length data
actually presentable in the data structure and saves memory too.

Add tests to show behavior with variable length signatures and missing
signatures.

Additionally update the signing code to store the largest IMA signature
length rather than what happened to be last to be on the safe side.
We can't rely on this value due to invalid packages being out there,
but then we need to calculate the lengths on rpmfiles populate so there's
not a lot to gain anyhow.

Fixes: rpm-software-management#1833
(cherry picked from commit 07f1d31)
dmnks pushed a commit to dmnks/rpm that referenced this issue Jun 7, 2022
…g:2018937)

At least ECDSA and RSA signatures can vary in length, but the IMA code
assumes constant lengths and thus may either place invalid signatures on
disk from either truncating or overshooting, and segfault if the stars are
just so.

As we can't assume static lengths and attempts to use maximum length
have proven problematic for other reasons, use a data structure that
can actually handle variable length data properly: store offsets into
the decoded binary blob and use them to calculate lengths when needed,
empty data is simply consequtive identical offsets. This avoids a whole
class of silly overflow issues with multiplying, makes zero-length data
actually presentable in the data structure and saves memory too.

Add tests to show behavior with variable length signatures and missing
signatures.

Additionally update the signing code to store the largest IMA signature
length rather than what happened to be last to be on the safe side.
We can't rely on this value due to invalid packages being out there,
but then we need to calculate the lengths on rpmfiles populate so there's
not a lot to gain anyhow.

Fixes: rpm-software-management#1833
(cherry picked from commit 07f1d31)
dmnks pushed a commit to dmnks/rpm that referenced this issue Jun 8, 2022
…g:2018937)

At least ECDSA and RSA signatures can vary in length, but the IMA code
assumes constant lengths and thus may either place invalid signatures on
disk from either truncating or overshooting, and segfault if the stars are
just so.

As we can't assume static lengths and attempts to use maximum length
have proven problematic for other reasons, use a data structure that
can actually handle variable length data properly: store offsets into
the decoded binary blob and use them to calculate lengths when needed,
empty data is simply consequtive identical offsets. This avoids a whole
class of silly overflow issues with multiplying, makes zero-length data
actually presentable in the data structure and saves memory too.

Add tests to show behavior with variable length signatures and missing
signatures.

Additionally update the signing code to store the largest IMA signature
length rather than what happened to be last to be on the safe side.
We can't rely on this value due to invalid packages being out there,
but then we need to calculate the lengths on rpmfiles populate so there's
not a lot to gain anyhow.

Fixes: rpm-software-management#1833
(cherry picked from commit 07f1d31)
dmnks pushed a commit that referenced this issue Jul 1, 2022
At least ECDSA and RSA signatures can vary in length, but the IMA code
assumes constant lengths and thus may either place invalid signatures on
disk from either truncating or overshooting, and segfault if the stars are
just so.

As we can't assume static lengths and attempts to use maximum length
have proven problematic for other reasons, use a data structure that
can actually handle variable length data properly: store offsets into
the decoded binary blob and use them to calculate lengths when needed,
empty data is simply consequtive identical offsets. This avoids a whole
class of silly overflow issues with multiplying, makes zero-length data
actually presentable in the data structure and saves memory too.

Add tests to show behavior with variable length signatures and missing
signatures.

Additionally update the signing code to store the largest IMA signature
length rather than what happened to be last to be on the safe side.
We can't rely on this value due to invalid packages being out there,
but then we need to calculate the lengths on rpmfiles populate so there's
not a lot to gain anyhow.

Fixes: #1833
(cherry picked from commit 07f1d31)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants