Fix the issue of going out of bounds in the isccfg parser. #1124

SandakovMM · 2023-08-24T13:02:54Z

This problem can occur when attempting to parse an empty file.

github-actions · 2023-08-24T13:03:06Z

Thank you for contributing to the Leapp project!

Please note that every PR needs to comply with the Leapp Guidelines and must pass all tests in order to be mergeable.
If you want to request a review or rebuild a package in copr, you can use following commands as a comment:

review please @oamg/developers to notify leapp developers of the review request
/packit copr-build to submit a public copr build using packit

Packit will automatically schedule regression tests for this PR's build and latest upstream leapp build. If you need a different version of leapp from PR#42, use /packit test oamg/leapp#42

To launch regression testing public members of oamg organization can leave the following comment:

/rerun to schedule basic regression tests using this pr build and latest upstream leapp build as artifacts
/rerun 42 to schedule basic regression tests using this pr build and leapp*PR42* as artifacts
/rerun-sst to schedule sst tests using this pr build and latest upstream leapp build as artifacts
/rerun-sst 42 to schedule sst tests using this pr build and leapp*PR42* as artifacts

Please open ticket in case you experience technical problem with the CI. (RH internal only)

Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please contact leapp-infra.

pirat89 · 2023-08-25T09:05:39Z

@SandakovMM thanks for the contribitioun. LGTM. @pemensik are you ok with the change?

pirat89 · 2023-08-25T09:06:04Z

/packit test

fernflower

Thanks for the fix! Could you please add a unit test?

SandakovMM · 2023-08-30T10:43:07Z

Could you please add a unit test?

Sure

SandakovMM · 2023-08-30T11:27:15Z

The test currently checks if the code will not crash. I'm not sure what else we could add to the check at this point. Please share your ideas if you have any.

pemensik

Core change is good, I would simplify the new test function a bit. And add additional include "/dev/null"; to test also included file check.

repos/system_upgrade/el7toel8/libraries/isccfg.py

pemensik · 2023-08-30T14:19:42Z

repos/system_upgrade/el7toel8/libraries/tests/test_isccfg.py

@@ -116,6 +116,8 @@
 };
 """)

+views_empty = isccfg.MockConfig("")
+


I would suggest also MockConfig('options { include "/dev/null"; };') testing, so we test also empty include file. I would rename it to config_empty variable, it contains no view.

pemensik · 2023-08-30T14:21:28Z

repos/system_upgrade/el7toel8/libraries/tests/test_isccfg.py

+def test_empty_config():
+    """ Test empty configuration """
+
+    callbacks = {


for empty file no callbacks make sense or are needed. Use just callbacks = {}.

pemensik · 2023-08-30T14:23:29Z

repos/system_upgrade/el7toel8/libraries/tests/test_isccfg.py

+        'dnssec-lookaside': cb_state,
+        'dnssec-validation': cb_state,
+    }
+    state = {}


Remove state usage, make it default to None. Unlike views test it is not needed here.

We can probably remove the state variable since it can be omitted from the walk call

pemensik · 2023-08-30T14:24:05Z

repos/system_upgrade/el7toel8/libraries/tests/test_isccfg.py

+    parser = isccfg.IscConfigParser(views_empty)
+    assert len(parser.FILES_TO_CHECK) == 1
+    cfg = parser.FILES_TO_CHECK[0]
+    parser.walk(cfg.root_section(), callbacks, state)


Omit 3rd parameter, not needed.

SandakovMM · 2023-09-06T12:31:47Z

How could we merge it?)

fernflower · 2023-09-06T13:03:56Z

@SandakovMM I could be mistaken, but I see that some comments from @pemensik have not been addressed (like https://github.com/oamg/leapp-repository/pull/1124/files/0f5b3da07b0ba3af50d88703e8620f54cd9af9be#diff-3755f648a6265f55301fda90f4333a3289285caa586c8bd8321c90d7746ae32bR361).

fernflower · 2023-09-06T13:21:01Z

My bad, all addressed, I will schedule tests and merge it when they are done. Thanks!

stale

fernflower · 2023-09-06T13:21:28Z

/packit test --labels sanity

fernflower · 2023-09-06T13:21:51Z

Just one thing - could you please rebase?

SandakovMM · 2023-09-06T14:15:30Z

Certainly, would you prefer to consolidate all the modifications into a single commit?

pirat89 · 2023-09-06T16:50:09Z

Certainly, would you prefer to consolidate all the modifications into a single commit?

@SandakovMM yes please :) thanks

fernflower · 2023-09-07T07:21:40Z

/packit test --labels sanity

fernflower · 2023-09-07T07:58:02Z

Could you please rebase against leapp-repository master? I'd like to run regression tests for this, but I need 93c6fd4 (and ~10 later commits that were delivered by #1110)

SandakovMM · 2023-09-07T15:03:42Z

Sure

fernflower · 2023-09-08T08:34:04Z

/packit test --labels sanity

fernflower · 2023-09-08T08:41:29Z

Either that is my bad and Friday speaking, or your branch https://github.com/SandakovMM/leapp-repository/tree/master is still missing the commits I need to enable testing :(
You need to rebase your branch against leapp-repository's master.

This problem can occur when attempting to parse an empty file.

SandakovMM · 2023-09-08T13:04:47Z

My bad, seem like I've rebased agains https://github.com/AlmaLinux/leapp-repository, not https://github.com/oamg/leapp-repository. Should be fine now.

fernflower · 2023-09-11T11:22:53Z

/packit test --labels sanity

fernflower

lgtm, when the tests pass I'll merge it.

pirat89 · 2023-09-13T17:18:30Z

@fernflower update please changelogs :)

fernflower · 2023-09-13T18:47:54Z

@pirat89 but I already did that 2 days ago..

pirat89 · 2023-09-25T17:56:02Z

@fernflower aah. so you just forgot to set the label :)

## Packaging - Requires xfsprogs and e2fsprogs (oamg#1154) - Bump leapp-repository-dependencies to 10 (oamg#1154) ## Upgrade handling ### Fixes - Detect changes in openssl default configuration file and restore it to the default to the target default during the upgrade to reduce risk of potential issues (oamg#1131) - Do not try to download data files anymore when missing as the service is obsoleted since the data is part of installed packages (oamg#1120) - Drop the invalid `tuv` target channel (oamg#1130) - Fix handling of symlinks under /etc/pki when managing certificates (oamg#1135, oamg#1160, oamg#1166) - Fix semanage import issue (oamg#1164) - Fix the issue of going out of bounds in the isccfg parser (oamg#1124) - Fix traceback when saving the rhsm facts results and the /etc/rhsm/facts directory doesn’t exist yet (oamg#1132) - Handle the upgrade better when a proxy is configured in YUM/DNF configutations (oamg#1143) - Load all rpm repository substitutions that dnf knows about, not just releasever since repofiles may use the other substitutions too (oamg#1134) - Minor updates of generated reports (oamg#1151) - Print nice error msg when device and driver deprecation data is malformed (oamg#1168) - Report information about required manual steps after the upgrade when openssl-ibmca is installed (oamg#1131) - Update error messages and reports when installed upgrade data files are malformed or missing (oamg#1120) - [IPU 7 -> 8] Fix the upgrade of the RH Satellite server when tomcat is installed (oamg#1150) - [IPU 8 -> 9] Fix the upgrade from RHEL 8.9+ when the release is locked by subscription-manager (oamg#1136, oamg#1138) ### Enhancements - Update upgrade paths: (oamg#1146, oamg#1147, oamg#1175) - RHEL 7.9 -> 8.10, 8.8 (default: 8.10) - RHEL with SAPAHA 7.9 -> 8.10, 8.8 (default: 8.8) - RHEL w/o SAP HANA 8.8 -> 9.2 - RHEL w/o SAP HANA 8.10 -> 9.4 - Added possibility to define DNF configuration for the target system (oamg#1143) - Code cleaning: drop redundant and invalid NFS checks (oamg#1127) - Default to NO_RHSM mode when subscription-manager is not found (oamg#1133) - Detect customized configuration of dynamic linker (oamg#1118) - Detect possible unexpected RPM GPG keys has been installed during RPM transaction (oamg#1101) - Drop obsoleted upgrade paths that relates to releases: 8.6, 8.9, 9.0, 9.3 (oamg#1175) - Ignore Leapp related PES events (oamg#1153) - Introduce generic transition of systemd services states during the IPU (oamg#1060, oamg#1174) - Introduce possibility to upgrade with local repositories (oamg#1099) - Introduced some changes getting us closer to possibility of IPU for Centos (Stream) systems (oamg#1140) - Report the upgrade customisations and modifications of the upgrade tooling (oamg#1148) - Simplify handling of upgrades on systems using RHUI, reducing the maintenance burden for cloud providers (oamg#1057) - Update the leapp upgrade data files - bump data stream to "3.0" (oamg#1163, oamg#1165, oamg#1170) - [IPU 8 -> 9] Enable upgrades RHEL 8 -> 9 using RHUI on Alibaba cloud (oamg#1137, oamg#1165, oamg#1172) ## Additional changes interesting for devels - Introduced new functions returning a list of packages related to upgrade - see the rpms library (oamg#1156) - Make detection of installed signed packages distribution agnostic - covers RHEL & CentOS (oamg#876) - Model InstalledRedHatSignedRPM is deprecated, replaced by DistributionSignedRPM (oamg#876)

## Packaging - Requires xfsprogs and e2fsprogs (oamg#1154) - Bump leapp-repository-dependencies to 10 (oamg#1154) ## Upgrade handling ### Fixes - Detect changes in openssl default configuration file and restore it to the default to the target default during the upgrade to reduce risk of potential issues (oamg#1131) - Do not try to download data files anymore when missing as the service is obsoleted since the data is part of installed packages (oamg#1120) - Drop the invalid `tuv` target channel (oamg#1130) - Fix handling of symlinks under /etc/pki when managing certificates (oamg#1135, oamg#1160, oamg#1166) - Fix semanage import issue (oamg#1164) - Fix the issue of going out of bounds in the isccfg parser (oamg#1124) - Fix traceback when saving the rhsm facts results and the /etc/rhsm/facts directory doesn’t exist yet (oamg#1132) - Handle the upgrade better when a proxy is configured in YUM/DNF configutations (oamg#1143) - Load all rpm repository substitutions that dnf knows about, not just releasever since repofiles may use the other substitutions too (oamg#1134) - Minor updates of generated reports (oamg#1151) - Print nice error msg when device and driver deprecation data is malformed (oamg#1168) - Report information about required manual steps after the upgrade when openssl-ibmca is installed (oamg#1131) - Update error messages and reports when installed upgrade data files are malformed or missing (oamg#1120) - [IPU 7 -> 8] Fix the upgrade of the RH Satellite server when tomcat is installed (oamg#1150) - [IPU 8 -> 9] Fix the upgrade from RHEL 8.9+ when the release is locked by subscription-manager (oamg#1136, oamg#1138) ### Enhancements - Update upgrade paths: (oamg#1146, oamg#1147, oamg#1175) - RHEL 7.9 -> 8.10, 8.8 (default: 8.10) - RHEL with SAPAHA 7.9 -> 8.10, 8.8 (default: 8.8) - RHEL w/o SAP HANA 8.8 -> 9.2 - RHEL w/o SAP HANA 8.10 -> 9.4 - Added possibility to define DNF configuration for the target system (oamg#1143) - Code cleaning: drop redundant and invalid NFS checks (oamg#1127) - Default to NO_RHSM mode when subscription-manager is not found (oamg#1133) - Detect customized configuration of dynamic linker (oamg#1118) - Detect possible unexpected RPM GPG keys has been installed during RPM transaction (oamg#1101) - Drop obsoleted upgrade paths that relates to releases: 8.6, 8.9, 9.0, 9.3 (oamg#1175) - Ignore Leapp related PES events (oamg#1153) - Introduce generic transition of systemd services states during the IPU (oamg#1060, oamg#1174) - Introduce possibility to upgrade with local repositories (oamg#1099) - Introduced some changes getting us closer to possibility of IPU for Centos (Stream) systems (oamg#1140) - Report the upgrade customisations and modifications of the upgrade tooling (oamg#1148) - Simplify handling of upgrades on systems using RHUI, reducing the maintenance burden for cloud providers (oamg#1057) - Update the leapp upgrade data files - bump data stream to "3.0" (oamg#1163, oamg#1165, oamg#1170) - [IPU 8 -> 9] Enable upgrades RHEL 8 -> 9 using RHUI on Alibaba cloud (oamg#1137, oamg#1165, oamg#1172) - Unify breakpoints inside the upgrade initramfs for the easier troubleshooting (oamg#1157) ## Additional changes interesting for devels - Introduced new functions returning a list of packages related to upgrade - see the rpms library (oamg#1156) - Make detection of installed signed packages distribution agnostic - covers RHEL & CentOS (oamg#876) - Model InstalledRedHatSignedRPM is deprecated, replaced by DistributionSignedRPM (oamg#876)

## Packaging - Requires xfsprogs and e2fsprogs (#1154) - Bump leapp-repository-dependencies to 10 (#1154) ## Upgrade handling ### Fixes - Detect changes in openssl default configuration file and restore it to the default to the target default during the upgrade to reduce risk of potential issues (#1131) - Do not try to download data files anymore when missing as the service is obsoleted since the data is part of installed packages (#1120) - Drop the invalid `tuv` target channel (#1130) - Fix handling of symlinks under /etc/pki when managing certificates (#1135, #1160, #1166) - Fix semanage import issue (#1164) - Fix the issue of going out of bounds in the isccfg parser (#1124) - Fix traceback when saving the rhsm facts results and the /etc/rhsm/facts directory doesn’t exist yet (#1132) - Handle the upgrade better when a proxy is configured in YUM/DNF configutations (#1143) - Load all rpm repository substitutions that dnf knows about, not just releasever since repofiles may use the other substitutions too (#1134) - Minor updates of generated reports (#1151) - Print nice error msg when device and driver deprecation data is malformed (#1168) - Report information about required manual steps after the upgrade when openssl-ibmca is installed (#1131) - Update error messages and reports when installed upgrade data files are malformed or missing (#1120) - [IPU 7 -> 8] Fix the upgrade of the RH Satellite server when tomcat is installed (#1150) - [IPU 8 -> 9] Fix the upgrade from RHEL 8.9+ when the release is locked by subscription-manager (#1136, #1138) ### Enhancements - Update upgrade paths: (#1146, #1147, #1175) - RHEL 7.9 -> 8.10, 8.8 (default: 8.10) - RHEL with SAPAHA 7.9 -> 8.10, 8.8 (default: 8.8) - RHEL w/o SAP HANA 8.8 -> 9.2 - RHEL w/o SAP HANA 8.10 -> 9.4 - Added possibility to define DNF configuration for the target system (#1143) - Code cleaning: drop redundant and invalid NFS checks (#1127) - Default to NO_RHSM mode when subscription-manager is not found (#1133) - Detect customized configuration of dynamic linker (#1118) - Detect possible unexpected RPM GPG keys has been installed during RPM transaction (#1101) - Drop obsoleted upgrade paths that relates to releases: 8.6, 8.9, 9.0, 9.3 (#1175) - Ignore Leapp related PES events (#1153) - Introduce generic transition of systemd services states during the IPU (#1060, #1174) - Introduce possibility to upgrade with local repositories (#1099) - Introduced some changes getting us closer to possibility of IPU for Centos (Stream) systems (#1140) - Report the upgrade customisations and modifications of the upgrade tooling (#1148) - Simplify handling of upgrades on systems using RHUI, reducing the maintenance burden for cloud providers (#1057) - Update the leapp upgrade data files - bump data stream to "3.0" (#1163, #1165, #1170) - [IPU 8 -> 9] Enable upgrades RHEL 8 -> 9 using RHUI on Alibaba cloud (#1137, #1165, #1172) - Unify breakpoints inside the upgrade initramfs for the easier troubleshooting (#1157) ## Additional changes interesting for devels - Introduced new functions returning a list of packages related to upgrade - see the rpms library (#1156) - Make detection of installed signed packages distribution agnostic - covers RHEL & CentOS (#876) - Model InstalledRedHatSignedRPM is deprecated, replaced by DistributionSignedRPM (#876)

## Packaging - Requires xfsprogs and e2fsprogs (oamg#1154) - Bump leapp-repository-dependencies to 10 (oamg#1154) ## Upgrade handling ### Fixes - Detect changes in openssl default configuration file and restore it to the default to the target default during the upgrade to reduce risk of potential issues (oamg#1131) - Do not try to download data files anymore when missing as the service is obsoleted since the data is part of installed packages (oamg#1120) - Drop the invalid `tuv` target channel (oamg#1130) - Fix handling of symlinks under /etc/pki when managing certificates (oamg#1135, oamg#1160, oamg#1166) - Fix semanage import issue (oamg#1164) - Fix the issue of going out of bounds in the isccfg parser (oamg#1124) - Fix traceback when saving the rhsm facts results and the /etc/rhsm/facts directory doesn’t exist yet (oamg#1132) - Handle the upgrade better when a proxy is configured in YUM/DNF configutations (oamg#1143) - Load all rpm repository substitutions that dnf knows about, not just releasever since repofiles may use the other substitutions too (oamg#1134) - Minor updates of generated reports (oamg#1151) - Print nice error msg when device and driver deprecation data is malformed (oamg#1168) - Report information about required manual steps after the upgrade when openssl-ibmca is installed (oamg#1131) - Update error messages and reports when installed upgrade data files are malformed or missing (oamg#1120) - [IPU 7 -> 8] Fix the upgrade of the RH Satellite server when tomcat is installed (oamg#1150) - [IPU 8 -> 9] Fix the upgrade from RHEL 8.9+ when the release is locked by subscription-manager (oamg#1136, oamg#1138) ### Enhancements - Update upgrade paths: (oamg#1146, oamg#1147, oamg#1175) - RHEL 7.9 -> 8.10, 8.8 (default: 8.10) - RHEL with SAPAHA 7.9 -> 8.10, 8.8 (default: 8.8) - RHEL w/o SAP HANA 8.8 -> 9.2 - RHEL w/o SAP HANA 8.10 -> 9.4 - Added possibility to define DNF configuration for the target system (oamg#1143) - Code cleaning: drop redundant and invalid NFS checks (oamg#1127) - Default to NO_RHSM mode when subscription-manager is not found (oamg#1133) - Detect customized configuration of dynamic linker (oamg#1118) - Detect possible unexpected RPM GPG keys has been installed during RPM transaction (oamg#1101) - Drop obsoleted upgrade paths that relates to releases: 8.6, 8.9, 9.0, 9.3 (oamg#1175) - Ignore Leapp related PES events (oamg#1153) - Introduce generic transition of systemd services states during the IPU (oamg#1060, oamg#1174) - Introduce possibility to upgrade with local repositories (oamg#1099) - Introduced some changes getting us closer to possibility of IPU for Centos (Stream) systems (oamg#1140) - Report the upgrade customisations and modifications of the upgrade tooling (oamg#1148) - Simplify handling of upgrades on systems using RHUI, reducing the maintenance burden for cloud providers (oamg#1057) - Update the leapp upgrade data files - bump data stream to "3.0" (oamg#1163, oamg#1165, oamg#1170) - [IPU 8 -> 9] Enable upgrades RHEL 8 -> 9 using RHUI on Alibaba cloud (oamg#1137, oamg#1165, oamg#1172) - Unify breakpoints inside the upgrade initramfs for the easier troubleshooting (oamg#1157) ## Additional changes interesting for devels - Introduced new functions returning a list of packages related to upgrade - see the rpms library (oamg#1156) - Make detection of installed signed packages distribution agnostic - covers RHEL & CentOS (oamg#876) - Model InstalledRedHatSignedRPM is deprecated, replaced by DistributionSignedRPM (oamg#876) (cherry picked from commit 6421225)

SandakovMM mentioned this pull request Aug 24, 2023

Bind configuration check actor got an stacktrace if one of file included from /etc/named.conf is empty AlmaLinux/leapp-repository#85

Open

pirat89 added this to the 8.10/9.4 milestone Aug 25, 2023

fernflower previously requested changes Aug 30, 2023

View reviewed changes

pemensik suggested changes Aug 30, 2023

View reviewed changes

pemensik mentioned this pull request Aug 30, 2023

Add isccfg manual running mode #1125

Merged

pemensik approved these changes Aug 31, 2023

View reviewed changes

SandakovMM force-pushed the master branch from 0e29a35 to 21c3f45 Compare September 7, 2023 04:13

pirat89 requested a review from fernflower September 7, 2023 07:56

SandakovMM force-pushed the master branch from 47f2582 to a72b70e Compare September 7, 2023 15:02

Fix the issue of going out of bounds in the isccfg parser.

3c6e189

This problem can occur when attempting to parse an empty file.

SandakovMM force-pushed the master branch from a72b70e to 3c6e189 Compare September 8, 2023 13:03

fernflower approved these changes Sep 11, 2023

View reviewed changes

fernflower merged commit 4d8ad1c into oamg:master Sep 11, 2023

pirat89 added the changelog-checked The merger/reviewer checked the changelog draft document and updated it when relevant label Sep 25, 2023

pirat89 mentioned this pull request Feb 13, 2024

Release 0.20.0 #1177

Merged

yuravk mentioned this pull request Aug 9, 2024

Update to version 0.20.0 AlmaLinux/leapp-repository#119

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix the issue of going out of bounds in the isccfg parser. #1124

Fix the issue of going out of bounds in the isccfg parser. #1124

SandakovMM commented Aug 24, 2023

github-actions bot commented Aug 24, 2023

pirat89 commented Aug 25, 2023

pirat89 commented Aug 25, 2023

fernflower left a comment

SandakovMM commented Aug 30, 2023 •

edited

Loading

SandakovMM commented Aug 30, 2023

pemensik left a comment

pemensik Aug 30, 2023

pemensik Aug 30, 2023

pemensik Aug 30, 2023

SandakovMM Aug 31, 2023

pemensik Aug 30, 2023

SandakovMM commented Sep 6, 2023

fernflower commented Sep 6, 2023

fernflower commented Sep 6, 2023

fernflower commented Sep 6, 2023

fernflower commented Sep 6, 2023

SandakovMM commented Sep 6, 2023

pirat89 commented Sep 6, 2023

fernflower commented Sep 7, 2023

fernflower commented Sep 7, 2023 •

edited

Loading

SandakovMM commented Sep 7, 2023

fernflower commented Sep 8, 2023

fernflower commented Sep 8, 2023

SandakovMM commented Sep 8, 2023

fernflower commented Sep 11, 2023

fernflower left a comment

pirat89 commented Sep 13, 2023

fernflower commented Sep 13, 2023

pirat89 commented Sep 25, 2023

Fix the issue of going out of bounds in the isccfg parser. #1124

Fix the issue of going out of bounds in the isccfg parser. #1124

Conversation

SandakovMM commented Aug 24, 2023

github-actions bot commented Aug 24, 2023

Thank you for contributing to the Leapp project!

pirat89 commented Aug 25, 2023

pirat89 commented Aug 25, 2023

fernflower left a comment

Choose a reason for hiding this comment

SandakovMM commented Aug 30, 2023 • edited Loading

SandakovMM commented Aug 30, 2023

pemensik left a comment

Choose a reason for hiding this comment

pemensik Aug 30, 2023

Choose a reason for hiding this comment

pemensik Aug 30, 2023

Choose a reason for hiding this comment

pemensik Aug 30, 2023

Choose a reason for hiding this comment

SandakovMM Aug 31, 2023

Choose a reason for hiding this comment

pemensik Aug 30, 2023

Choose a reason for hiding this comment

SandakovMM commented Sep 6, 2023

fernflower commented Sep 6, 2023

fernflower commented Sep 6, 2023

fernflower commented Sep 6, 2023

fernflower commented Sep 6, 2023

SandakovMM commented Sep 6, 2023

pirat89 commented Sep 6, 2023

fernflower commented Sep 7, 2023

fernflower commented Sep 7, 2023 • edited Loading

SandakovMM commented Sep 7, 2023

fernflower commented Sep 8, 2023

fernflower commented Sep 8, 2023

SandakovMM commented Sep 8, 2023

fernflower commented Sep 11, 2023

fernflower left a comment

Choose a reason for hiding this comment

pirat89 commented Sep 13, 2023

fernflower commented Sep 13, 2023

pirat89 commented Sep 25, 2023

SandakovMM commented Aug 30, 2023 •

edited

Loading

fernflower commented Sep 7, 2023 •

edited

Loading