Check that i18nstrings are valid and consistent

QubesOS / qubes-rpm-oxide

Rust libraries for processing RPM packages

4 stars 4 forks source link

Check that i18nstrings are valid and consistent #14

Closed DemiMarie closed 2 years ago

DemiMarie commented 2 years ago

RPM assumes that the i18ntable has the same length as all i18nstring entries, and all released versions may read out of bounds if the i18ntable is too short. Reject packages that have any of the following:

An i18ntable entry or any i18nstring entries in the signature header.
An i18ntable entry with a type other than RPM_STRING_ARRAY_TYPE, which would be unusable.
An i18nstring entry in the main header that is not preceeded by an i18ntable entry. Due to tag ordering, the i18ntable should always preceed any i18nstring entries.
An i18nstring entry with a different length than the preceeding i18ntable.

Tests included.

marmarek commented 2 years ago

I think the code is fine, but indeed it could use some tests, including malformed package.

DemiMarie commented 2 years ago

I think the code is fine, but indeed it could use some tests, including malformed package.

I think a test package is less important than a convenient way to generate malformed packages and malformed headers. Spending hours crafting each test case with a bespoke program simply will not scale.

DemiMarie commented 2 years ago

I think the code is fine, but indeed it could use some tests, including malformed package.

Integration test added. I didn’t bother creating a full package, which would have been much more work, and instead tested the parsing function directly.

DemiMarie commented 2 years ago

PipelineRetry

DemiMarie commented 2 years ago

PipelineRetry

DemiMarie commented 2 years ago

PipelineRetry

DemiMarie commented 2 years ago

PipelineRetry

marmarek commented 2 years ago

@DemiMarie

Jul 08 19:45:38 dom0 qubes.ReceiveUpdates+-sys-firewall[36880]: Error canonicalizing file: Bad length for i18nstring TagData { tag: 4160946176, ty: 150994944, offset: 2567045120, count: 16777216 }: expected 2 but got 1
Jul 08 19:45:38 dom0 qrexec-policy-daemon[36879]: Error canonicalizing file: Bad length for i18nstring TagData { tag: 4160946176, ty: 150994944, offset: 2567045120, count: 16777216 }: expected 2 but got 1
Jul 08 19:45:38 dom0 qubes.ReceiveUpdates+-sys-firewall[36880]: Error canonicalizing /var/tmp/qubes-updates-tmpk8lvilk_.UNTRUSTED/gnat-srpm-macros-4-11.fc32.noarch.rpm
Jul 08 19:45:38 dom0 qrexec-policy-daemon[36879]: Error canonicalizing /var/tmp/qubes-updates-tmpk8lvilk_.UNTRUSTED/gnat-srpm-macros-4-11.fc32.noarch.rpm

I reverted the commit.

DemiMarie commented 2 years ago

@DemiMarie

Jul 08 19:45:38 dom0 qubes.ReceiveUpdates+-sys-firewall[36880]: Error canonicalizing file: Bad length for i18nstring TagData { tag: 4160946176, ty: 150994944, offset: 2567045120, count: 16777216 }: expected 2 but got 1
Jul 08 19:45:38 dom0 qrexec-policy-daemon[36879]: Error canonicalizing file: Bad length for i18nstring TagData { tag: 4160946176, ty: 150994944, offset: 2567045120, count: 16777216 }: expected 2 but got 1
Jul 08 19:45:38 dom0 qubes.ReceiveUpdates+-sys-firewall[36880]: Error canonicalizing /var/tmp/qubes-updates-tmpk8lvilk_.UNTRUSTED/gnat-srpm-macros-4-11.fc32.noarch.rpm
Jul 08 19:45:38 dom0 qrexec-policy-daemon[36879]: Error canonicalizing /var/tmp/qubes-updates-tmpk8lvilk_.UNTRUSTED/gnat-srpm-macros-4-11.fc32.noarch.rpm

I reverted the commit.

Whoops, that is too strict. It’s fine for the i18nstring to be shorter than the i18ntable; it just must not be longer.