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

Fix regex string for **Del and **DelVals #56060

Merged
merged 3 commits into from
Mar 11, 2020
Merged

Conversation

twangboy
Copy link
Contributor

@twangboy twangboy commented Feb 4, 2020

What does this PR do?

The regex string that updates the Registry.pol file wasn't properly matching instances where the reg_key was prepended with **Del or **DelVals due to the utf-16-le encoding of the Registry.pol file.

This adds the null byte characters to the regex portion that searches for those values.

Related issue

#56062

Previous Behavior

The example I was working with was the Default Consent key which renders in the Registry.pol file like this: "D e f a u l t C o n s e n t". Where the spaces between the characters are null bytes. (utf-16-le)

When this policy is disabled there is some additional text prepended, like this: "* * d e l . D e f a u l t C o n s e n t"

The original regex did not include the null byte characters and therefore could not find the policy that needed to be changed, so it appended the new setting to the file. This caused there to be two conflicting entries in the file for the same setting.

New Behavior

Null byte characters are accounted for and you can now set this policy.

Tests written?

Yes

Commits signed with GPG?

Yes

@twangboy twangboy requested a review from a team as a code owner February 4, 2020 21:47
@ghost ghost requested a review from Akm0d February 4, 2020 21:48
@sagetherage sagetherage linked an issue Feb 5, 2020 that may be closed by this pull request
@twangboy
Copy link
Contributor Author

@lomeroe Would you mind taking a look at this?

Copy link
Contributor

@lomeroe lomeroe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

The regex string that updates the Registry.pol file wasn't properly
matching instances where the reg_key was prepended with **Del or
**DelVals due to the utf-16-le encoding of the Registry.pol file.

This adds the null byte characters to the regex portion that searches
for those values.
@dwoz dwoz merged commit e551ff6 into saltstack:master Mar 11, 2020
@lorengordon
Copy link
Contributor

Heh, thanks for this @twangboy! I was just troubleshooting the same issue, but in the _policyFileReplaceOrAppend and _policyFileReplaceOrAppendList functions. My solution was a little more... violent. I just redefined the function in my own custom module, using pure re functionality...

def _policyFileReplaceOrAppend(policy, policy_data, append=True):
    '''
    helper function to take a ADMX policy string for registry.pol file data and
    update existing string or append the string to the data

    Cut from win_lgpo.py due to bugs when there are DELETE policies
    '''
    # token to match policy start = encoded [
    policy_start = re.escape('['.encode('utf-16-le'))

    # token to match policy end = encoded ]
    policy_end = re.escape(']'.encode('utf-16-le'))

    # token to match policy delimiter = encoded null + encoded semicolon
    policy_delimiter = b''.join([
        chr(0).encode('utf-16-le'),
        ';'.encode('utf-16-le'),
    ])

    # pattern group to OPTIONALLY match delete instructions in value token
    policy_pattern_del = b''.join([
        '(',
        re.escape('**Del.'.encode('utf-16-le')),
        '|',
        re.escape('**DelVals.'.encode('utf-16-le')),
        '){0,1}',
    ])

    # pattern group to match one delimited token in a policy
    policy_token = b''.join([
        '(',
        '.*?',  # non-greedy match, up to next policy delimiter
        policy_delimiter,
        ')',
    ])

    # pattern to capture the key and value tokens from `policy`
    policy_pattern = b''.join([
        policy_start,
        policy_token, # this is the registry key
        policy_pattern_del,
        policy_token, # this is the value
        '.*?',        # this is the remainder of the policy tokens
        policy_end,
    ])

    # parse the tokens from `policy`
    policy_match = re.search(
        policy_pattern,
        policy,
        flags=re.IGNORECASE|re.DOTALL,
    )

    # pattern to match `policy` in `policy_data`
    policy_match_groups = policy_match.groups()
    policy_data_pattern = b''.join([
        policy_start,
        re.escape(policy_match_groups[0]),  # key
        policy_pattern_del,
        re.escape(policy_match_groups[2]),  # value
        '.*?',
        policy_end,
    ])

    # search for `policy` in `policy_data`
    policy_data_match = re.search(
        policy_data_pattern,
        policy_data,
        flags=re.IGNORECASE|re.DOTALL,
    )

    if policy_data_match:
        # replace a match with the policy
        policy_repl = policy_data_match.group()
        log.debug('replacing "%s" with "%s"', policy_repl, policy)
        return policy_data.replace(policy_repl, policy)
    elif append:
        # append the policy
        log.debug('appending "%s"', policy)
        return b''.join([policy_data, policy])
    else:
        # no match, no append, just return what we were given
        return policy_data

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v3000.1 vulnerable version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LGPO Module doesn't handle **Del and **DelVals properly
7 participants