Skip to content

Potential overflow fixed#246

Open
ayushman1210 wants to merge 8 commits intoPecanProject:masterfrom
ayushman1210:overflow_fixed
Open

Potential overflow fixed#246
ayushman1210 wants to merge 8 commits intoPecanProject:masterfrom
ayushman1210:overflow_fixed

Conversation

@ayushman1210
Copy link
Contributor

Location: src/sipnet/events.c, line 183

Description:

#define EVENT_LINE_SIZE 1024
char line[EVENT_LINE_SIZE];

When reading event lines, the buffer is fixed at 1024 bytes, but there's no check to ensure event parameter strings don't exceed this size.

Issue:
While fgets() will prevent buffer overflow, extremely long event parameter lines will be silently truncated, potentially causing:

  • Incorrect parsing of event data
  • Missing event parameters
  • Silent data corruption

Impact:

  • Events with very detailed parameters might be incorrectly processed
  • Difficult to debug because no error is reported

Changes
Add validation after reading

if (fgets(line, EVENT_LINE_SIZE, in) != NULL) {
  size_t len = strlen(line);
  if (len == EVENT_LINE_SIZE - 1 && line[len-1] != '\n') {
    logError("Event line too long (exceeds %d chars), data may be truncated\n", 
             EVENT_LINE_SIZE);
    exit(EXIT_CODE_INPUT_FILE_ERROR);
  }
}

@ayushman1210 ayushman1210 force-pushed the overflow_fixed branch 2 times, most recently from 07cb827 to 6b19abe Compare February 1, 2026 15:57
@ayushman1210
Copy link
Contributor Author

Hi @dlebauer @Alomir just wanted to check in and see if you had any thoughts on this PR when you get a chance. Happy to make changes or answer questions. Thanks!!

@Alomir Alomir changed the title Overflow fixed Potential overflow fixed Feb 2, 2026
Copy link
Collaborator

@Alomir Alomir left a comment

Choose a reason for hiding this comment

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

Looks good, just a small suggestion for change

@ayushman1210 ayushman1210 requested a review from Alomir February 3, 2026 18:12
Copy link
Collaborator

@Alomir Alomir left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants