Skip to content

Commit

Permalink
[7859] Do not allow null pointer to be returned from msiDataObjChksum.
Browse files Browse the repository at this point in the history
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 committed Jul 2, 2024
1 parent 878f963 commit 9d67f85
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 1 deletion.
35 changes: 35 additions & 0 deletions scripts/irods/test/test_all_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -1818,6 +1818,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:
attribute_name = 'issue_7859_checksum'

# Add a rule that attempts to verify the checksum using msiDataObjChksum.
# Upon success, an AVU will be added to the data object. The attribute value
# should be an empty string, signaling that msiDataObjChksum detected the null
# pointer and converted it to an empty string appropriately.
core.add_rule(dedent(f'''
pep_api_data_obj_trim_pre(rule_args, callback, rei):
logical_path = str(rule_args[0].objPath)
result = callback.msiDataObjChksum(logical_path, 'verifyChksum=', '')
callback.msiModAVUMetadata('-d', logical_path, 'set', '{attribute_name}', result['arguments'][2], '')
''')

# Create a new data object.
# The data object will be removed automatically by the test framework.
data_object = 'issue_7859_data_object'
self.user0.assert_icommand(['itouch', data_object])

# Calculate a checksum.
# This is important because the checksum verification performed by the
# microservice will conclude that the replica is in good standing.
self.user0.assert_icommand(['ichksum', data_object], 'STDOUT', ['sha2:47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU='])

# Trigger the PEP.
# We don't care about the result of the command. However, we do expect the
# itrim operation to fail because there's only one replica for the data object.
self.user0.assert_icommand_fail(['itrim', data_object])

# If the AVU set by the PEP exists on the data object, then everything is
# working as intended.
query_string = f"select DATA_NAME where META_DATA_ATTR_NAME = '{attribute_name}' and META_DATA_ATTR_VALUE = ''"
self.user0.assert_icommand( ['iquest', query_string], '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 9d67f85

Please sign in to comment.