Skip to content

Commit

Permalink
[irods#7859] Convert null pointer to empty string for checksum output…
Browse files Browse the repository at this point in the history
… parameter of msiDataObjChksum.

Before this commit, the following conditions could lead to a segfault.

    - The python rule engine plugin is enabled
    - A rule written in python invokes msiDataObjChksum
    - Checksum verification is performed and no issues are found

The segfault is the result of the microservice setting the checksum
output parameter to a null pointer (because it did not find any issues)
which eventually leads to iRODS attempting to construct a std::string
with the null pointer.

The construction of the std::string occurs in convertFromMsParam.
Passing a null pointer to the std::string constructor is undefined
behavior.
  • Loading branch information
korydraughn authored and alanking committed Jul 4, 2024
1 parent 0295555 commit e4b196d
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 1 deletion.
36 changes: 36 additions & 0 deletions scripts/irods/test/test_all_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from .. import paths
from ..configuration import IrodsConfig
from ..controller import IrodsController
from ..core_file import temporary_core_file
from . import metaclass_unittest_test_case_generator
from . import resource_suite
from . import session
Expand Down Expand Up @@ -1818,6 +1819,41 @@ def test_msiSetKeyValuePairsToObj_does_not_crash__issue_7027(self):

self.admin.assert_icommand(['imeta', 'ls', '-d', TEST_FILE], 'STDOUT', [ 'attribute: key\nvalue: val\n' ])

@unittest.skipUnless(plugin_name == 'irods_rule_engine_plugin-python', "Requires the PREP.")
def test_msiDataObjChksum_does_not_lead_to_segfault_on_good_verification_result__issue_7859(self):
with temporary_core_file() as core:
attr_name = 'issue_7859'
attr_value = 'issue_7859 is working'

# Add a rule that attempts to verify the checksum using msiDataObjChksum.
# If verification finds no issues, the checksum output variable will hold an empty string.
# The check for the empty string and attachment of metadata give the test a way to determine
# the outcome of the rule.
core.add_rule(textwrap.dedent(f'''
def pep_api_data_obj_trim_pre(rule_args, callback, rei):
logical_path = str(rule_args[2].objPath)
result = callback.msiDataObjChksum(logical_path, 'verifyChksum=', '')
if result['arguments'][2] == '':
callback.msiModAVUMetadata('-d', logical_path, 'set', '{attr_name}', '{attr_value}', '')
'''))

# Create a new data object.
data_object = 'issue_7859_data_object'
self.user0.assert_icommand(['itouch', data_object])

# Calculate a checksum so the rule has information it can use for verification.
# Without this, the rule will fail.
self.user0.assert_icommand(['ichksum', data_object], 'STDOUT', ['sha2:47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU='])

# Trigger the PEP.
# Before the fix, this would result in an error.
self.user0.assert_icommand(['itrim', data_object], 'STDOUT', ['Number of files trimmed = 0.'])

# Show the checksum verification was successful.
self.user0.assert_icommand(
['iquest', f"select DATA_NAME where META_DATA_ATTR_NAME = '{attr_name}' and META_DATA_ATTR_VALUE = '{attr_value}'"],
'STDOUT',
[f'DATA_NAME = {data_object}\n'])

class Test_msiDataObjRepl_checksum_keywords(session.make_sessions_mixin([('otherrods', 'rods')], [('alice', 'apass')]), unittest.TestCase):
global plugin_name
Expand Down
2 changes: 1 addition & 1 deletion server/re/src/reDataObjOpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1621,7 +1621,7 @@ msiDataObjChksum( msParam_t *inpParam1, msParam_t *msKeyValStr,
}

if ( rei->status >= 0 ) {
fillStrInMsParam( outParam, chksum );
fillStrInMsParam(outParam, chksum ? chksum : "");
free( chksum );
}
else {
Expand Down

0 comments on commit e4b196d

Please sign in to comment.