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

fvwm/session.c has left FVWM_SM_DEBUG_FILES enabled #480

Closed
pm215 opened this issue May 2, 2021 · 4 comments · Fixed by #483
Closed

fvwm/session.c has left FVWM_SM_DEBUG_FILES enabled #480

pm215 opened this issue May 2, 2021 · 4 comments · Fixed by #483
Assignees
Labels
type:bug Something's broken!
Milestone

Comments

@pm215
Copy link
Contributor

pm215 commented May 2, 2021

In fvwm/session.c the debug definition FVWM_SM_DEBUG_FILES is defined, which means that whenever it saves or reloads window states it copies state files into /tmp/fs-save and /tmp/fs-load directories.

This seems to me like an accidentally left-enabled bit of debug (it was enabled in commit 974902a in 2016, and that commit's commit message doesn't mention enabling the debug as a deliberate choice).

Should the #define be commented out like the other FVWM_SM_DEBUG* lines ?

(I noticed this because the system() calls in the debug code trigger a compile warning because they don't do anything with the return value from system().)

@pm215 pm215 added the type:bug Something's broken! label May 2, 2021
@ThomasAdam
Copy link
Member

Hi @pm215

It's a little dirty, but very useful for debugging, which is why I probably left it enabled all those years ago. We could probably harden the system() check though, but I rather this stays.

@pm215
Copy link
Contributor Author

pm215 commented May 2, 2021

I'd definitely rather my window manager wasn't writing debug info to fixed-filename temporary files in /tmp. If these have a good purpose they ought to be somewhere fvwm-specific, surely ?

@ThomasAdam
Copy link
Member

Then we should change it to be relative to FVWM_USERDIR.

@ThomasAdam ThomasAdam reopened this May 8, 2021
@ThomasAdam ThomasAdam self-assigned this May 8, 2021
@ThomasAdam ThomasAdam added this to the 1.0.3 milestone May 8, 2021
ThomasAdam added a commit that referenced this issue May 8, 2021
When session state is being stored, the debug macro would copy over
these files to /tmp.  The security-conscious would say this is risky,
especially since system() is being used.

For now, completely remove this, and if there's any further debugging
required, some other mechanism can be used.

Fixes #480
@ThomasAdam
Copy link
Member

I'm just going to remove this for now -- if we ever need to do anything else, we can code for that.

ThomasAdam added a commit that referenced this issue May 8, 2021
When session state is being stored, the debug macro would copy over
these files to /tmp.  The security-conscious would say this is risky,
especially since system() is being used.

For now, completely remove this, and if there's any further debugging
required, some other mechanism can be used.

Fixes #480
@ThomasAdam ThomasAdam added this to FVWM3 Sep 18, 2022
@ThomasAdam ThomasAdam moved this to Done in FVWM3 Sep 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something's broken!
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants