github-starred/firejail

Fork 0

mirror of https://github.com/netblue30/firejail.git synced 2026-05-15 14:16:14 -06:00

[GH-ISSUE #6792] potential double-free in `procevent_monitor` (SAST Warning) #3373

New issue

Closed

opened 2026-05-05 09:56:56 -06:00 by gitea-mirror · 5 comments

gitea-mirror commented

2026-05-05 09:56:56 -06:00

Owner

Originally created by @grey3228 on GitHub (Jun 26, 2025).
Original GitHub issue: https://github.com/netblue30/firejail/issues/6792

Hello!

I used the SAST tool Svace to analyze Firejail (v0.9.72) and encountered a warning about a potential double-free in src/firemon/procevent.c. I'd like to verify whether this is a false positive or a legitimate issue.

Warning Details

In procevent_monitor, the tool flags a scenario where cmd might be freed twice:

First free (Line 456):

else {
	sprintf(lineptr, " %s\n", cmd);
	free(cmd);  // First deallocation
}

Second free (Line 471):

// unflag pid for exit events
if (remove_pid) {
	if (pids[pid].option.event.user)
		free(pids[pid].option.event.user);
	if (pids[pid].option.event.cmd)
		free(pids[pid].option.event.cmd); // Potential second deallocation
	memset(&pids[pid], 0, sizeof(Process));
}

Concern

The double-free could occur if:

pids[pid].option.event.cmd != NULL (i.e., a command was previously cached);
remove_pid != 0 (i.e., the process is exiting);
Control flow passes through both code blocks (e.g., for a non-Firejail process exit event).

Proposed Fix

To prevent this, we could add a check before freeing cmd:

else {
	sprintf(lineptr, " %s\n", cmd);
	if (cmd != pids[pid].option.event.cmd) {  // Only free if not cached
		free(cmd);
	}
}

Questions

Is this a false positive, or does a valid execution path trigger this issue?
If it's a real issue, would the proposed fix be acceptable?

Thank you for your time and expertise!

Originally created by @grey3228 on GitHub (Jun 26, 2025). Original GitHub issue: https://github.com/netblue30/firejail/issues/6792 Hello! I used the SAST tool **Svace** to analyze Firejail (v0.9.72) and encountered a warning about a potential **double-free** in `src/firemon/procevent.c`. I'd like to verify whether this is a false positive or a legitimate issue. ### Warning Details In `procevent_monitor`, the tool flags a scenario where `cmd` might be freed twice: 1. **First free** (Line 456): ```c else { sprintf(lineptr, " %s\n", cmd); free(cmd); // First deallocation } ``` 2. **Second free** (Line 471): ```c // unflag pid for exit events if (remove_pid) { if (pids[pid].option.event.user) free(pids[pid].option.event.user); if (pids[pid].option.event.cmd) free(pids[pid].option.event.cmd); // Potential second deallocation memset(&pids[pid], 0, sizeof(Process)); } ``` ### Concern The double-free could occur if: * pids[pid].option.event.cmd != NULL (i.e., a command was previously cached); * remove_pid != 0 (i.e., the process is exiting); * Control flow passes through both code blocks (e.g., for a non-Firejail process exit event). ### Proposed Fix To prevent this, we could add a check before freeing cmd: ```c else { sprintf(lineptr, " %s\n", cmd); if (cmd != pids[pid].option.event.cmd) { // Only free if not cached free(cmd); } } ``` ### Questions 1. Is this a false positive, or does a valid execution path trigger this issue? 2. If it's a real issue, would the proposed fix be acceptable? Thank you for your time and expertise!

gitea-mirror

2026-05-05 09:56:56 -06:00

closed this issue
added the
bug
label

gitea-mirror commented

2026-05-05 09:56:58 -06:00

Author

Owner

@kmk3 commented on GitHub (Jul 1, 2025):

Thanks for reporting this.

This is a preliminary response as I haven't finished looking into this, but
after a brief look at the current code, your analysis of the problem seems to
be correct.

v0.9.72

Note that we do not maintain that version of firejail:

https://github.com/netblue30/firejail/blob/master/SECURITY.md

Versions other than the latest usually have outdated profiles and may contain
bugs and security vulnerabilities that were fixed in later versions.

See also:

https://github.com/netblue30/firejail#installing

Is there a specific reason that you're using 0.9.72?

@kmk3 commented on GitHub (Jul 1, 2025): Thanks for reporting this. This is a preliminary response as I haven't finished looking into this, but after a brief look at the current code, your analysis of the problem seems to be correct. > v0.9.72 Note that we do not maintain that version of firejail: * <https://github.com/netblue30/firejail/blob/master/SECURITY.md> Versions other than the latest usually have outdated profiles and may contain bugs and security vulnerabilities that were fixed in later versions. See also: * <https://github.com/netblue30/firejail#installing> Is there a specific reason that you're using 0.9.72?

gitea-mirror commented

2026-05-05 09:56:59 -06:00

Author

Owner

@grey3228 commented on GitHub (Jul 2, 2025):

@kmk3 , thank you for your response!

I worked on a system with installed firejail package of version v0.9.72, so I tried to analyze this particular version. As I see, same potential issue exists in newer version. If this issue will be confirmed, it would be great to get fix in newer supported versions anyway.

@grey3228 commented on GitHub (Jul 2, 2025): @kmk3 , thank you for your response! I worked on a system with installed firejail package of version v0.9.72, so I tried to analyze this particular version. As I see, same potential issue exists in newer version. If this issue will be confirmed, it would be great to get fix in newer supported versions anyway.

gitea-mirror commented

2026-05-05 09:57:00 -06:00

Author

Owner

@grey3228 commented on GitHub (Jul 31, 2025):

@kmk3 hello! Sorry for disturbing. If this is a bug, do you mind if I create PR with proposed fix or you prefer to fix it by yourself?

@grey3228 commented on GitHub (Jul 31, 2025): @kmk3 hello! Sorry for disturbing. If this is a bug, do you mind if I create PR with proposed fix or you prefer to fix it by yourself?

gitea-mirror commented

2026-05-05 09:57:01 -06:00

Author

Owner

@kmk3 commented on GitHub (Jul 31, 2025):

hello! Sorry for disturbing.

All good, I was planning on following up on this soon.

If this is a bug, do you mind if I create PR with proposed fix or you prefer
to fix it by yourself?

Last time I looked at this, it seemed to be a real bug.

IIRC I had tried to come up with something slightly different but after working
on the code for a while your proposed fix still seemed better in the end, so
please open a PR.

@kmk3 commented on GitHub (Jul 31, 2025): > hello! Sorry for disturbing. All good, I was planning on following up on this soon. > If this is a bug, do you mind if I create PR with proposed fix or you prefer > to fix it by yourself? Last time I looked at this, it seemed to be a real bug. IIRC I had tried to come up with something slightly different but after working on the code for a while your proposed fix still seemed better in the end, so please open a PR.

gitea-mirror commented

2026-05-05 09:57:02 -06:00

Author

Owner

@grey3228 commented on GitHub (Jul 31, 2025):

Ok, thanks, I'll open PR soon then!

@grey3228 commented on GitHub (Jul 31, 2025): Ok, thanks, I'll open PR soon then!

gitea-mirror referenced this issue

2026-05-05 10:25:42 -06:00

[PR #3373] [MERGED] update --build #4739

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/firejail#3373

No description provided.

Rows
Columns

[GH-ISSUE #6792] potential double-free in procevent_monitor (SAST Warning) #3373

Warning Details

Concern

Proposed Fix

Questions

[GH-ISSUE #6792] potential double-free in `procevent_monitor` (SAST Warning) #3373