Skip to content

Fix for NAS-Filter-Rule hack#2

Open
fdurand wants to merge 1 commit intofeature/PacketFence_3.2.7from
fix/nas-filter-rule
Open

Fix for NAS-Filter-Rule hack#2
fdurand wants to merge 1 commit intofeature/PacketFence_3.2.7from
fix/nas-filter-rule

Conversation

@fdurand
Copy link
Member

@fdurand fdurand commented May 12, 2025

Fixes NAS-Filter-Rule attribute when the size of the value is bigger than the attribute size.

@fdurand fdurand requested a review from jrouzierinverse May 12, 2025 14:42
@fdurand fdurand added the bug Something isn't working label May 12, 2025
@fdurand fdurand requested a review from Copilot May 12, 2025 14:43
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes an issue with the NAS-Filter-Rule attribute when the rule value size exceeds the attribute size by aggregating multiple value pairs into a temporary buffer before output.

  • Introduces a rule_buffer and rule_len to accumulate rule data.
  • Replaces iteration over vp with a new pointer (current) and processes the buffered data in chunks with appropriate header resets.
  • Adjusts pointer assignments to properly update the VALUE_PAIR pointer.
Comments suppressed due to low confidence (1)

src/lib/radius.c:1690

  • [nitpick] The flag 'zero' is ambiguous; consider renaming it to 'insert_separator' to clarify its purpose of indicating when a separator byte should be added between concatenated rules.
zero = false;

Comment on lines +1645 to 1651
uint8_t rule_buffer[1024] = {0};
size_t rule_len = 0;
VALUE_PAIR *current = vp;

attr[0] = PW_NAS_FILTER_RULE;
attr[1] = 2;
p = ptr + 2;

while (vp && !vp->da->vendor && (vp->da->attr == PW_NAS_FILTER_RULE)) {
if ((p + zero + vp->vp_length) > end) {
while (current && !current->da->vendor && (current->da->attr == PW_NAS_FILTER_RULE)) {
if ((rule_len + current->vp_length + 1) >= sizeof(rule_buffer)) {
break;
Copy link

Copilot AI May 12, 2025

Choose a reason for hiding this comment

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

Using a fixed-size rule_buffer may silently truncate data when the accumulated length exceeds its capacity; consider implementing explicit error logging or dynamic buffer resizing to handle such cases.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Comments