You are not logged in.

#1 2025-08-26 12:53:56

Lone_Wolf
Administrator
From: Netherlands, Europe
Registered: 2005-10-04
Posts: 14,310

[Solved] fixing format-secuirty errors

I'm trying to help to improve oolite sourcecode and felt the best way to start is to eliminate the need to build with -Wno-format-security.

The first error when building oolite-git from AUR without that flag is

src/SDL/main.m: In function ‘main’:
src/SDL/main.m:171:45: error: format not a string literal and no format arguments [-Werror=format-security]
  171 |                                 OO_SHOW_MSG(s, processName, MB_OK);
      |                                             ^
src/SDL/main.m:119:73: note: in definition of macro ‘OO_SHOW_MSG’
  119 |         #define OO_SHOW_MSG(ooMsg, ooTitle, ooFlags)    fprintf(stdout, ooMsg)
      |                                                                         ^~~~~
cc1obj: some warnings being treated as errors

The file in q is https://github.com/OoliteProject/oolite … SDL/main.m .

I do understand that formatting should be added but have no idea which format specifier would work.
Please help.

Last edited by Lone_Wolf (2025-08-31 10:48:45)


Disliking systemd intensely, but not satisfied with alternatives so focusing on taming systemd.

clean chroot building not flexible enough ?
Try clean chroot manager by graysky

Offline

#2 2025-08-26 13:59:55

seth
Member
From: Don't DM me only for attention
Registered: 2012-09-03
Posts: 69,058

Re: [Solved] fixing format-secuirty errors

fprintf(stdout, "%s", ooMsg)

Not sure whether the string literal requirement is entirely correct on gcc's part, though.

Online

#3 2025-08-26 15:07:05

lmn
Member
Registered: 2021-05-09
Posts: 86
Website

Re: [Solved] fixing format-secuirty errors

This specific warning is thrown because the compiler can not check whether the resulting format string is valid (in accordance with the supplied arguments).
In this case the process name could contain a format specifier e.g. %s and cause fprintf to format an empty parameter list which pops a random value off the stack.
Try this example:

printf("%s\n");

As snprintf writes the null byte in the buffer it should not overflow but could truncate the output if the process name is to long.

Also the other parameters are silently discarded, which is fine but could be added for consistency.

It is not a requirement hence why it throws a warning.

Offline

#4 2025-08-26 21:14:50

loqs
Member
Registered: 2014-03-06
Posts: 18,628

Re: [Solved] fixing format-secuirty errors

lmn wrote:

This specific warning is thrown because the compiler can not check whether the resulting format string is valid (in accordance with the supplied arguments).
In this case the process name could contain a format specifier e.g. %s and cause fprintf to format an empty parameter list which pops a random value off the stack.

lmn wrote:

Also the other parameters are silently discarded, which is fine but could be added for consistency.

src/SDL/main.m:171:45: error: format not a string literal and no format arguments [-Werror=format-security]
  171 |                                 OO_SHOW_MSG(s, processName, MB_OK);

I think you have s and processName mixed up.  s is the output from snprintf https://github.com/OoliteProject/oolite … ain.m#L150 so is already length checked and null terminated. From that it could be argued to just ignore the warning for this specific instance.

Offline

#5 2025-08-26 21:45:43

Lone_Wolf
Administrator
From: Netherlands, Europe
Registered: 2005-10-04
Posts: 14,310

Re: [Solved] fixing format-secuirty errors

It is not a requirement hence why it throws a warning.

Archlinux default flags include -Werror=format-security so it's no longer considered a warning.

OO_SHOW_MSG is  a macro and defined on line 117 of the file

	#define OO_SHOW_MSG(ooMsg, ooTitle, ooFlags)	fprintf(stdout, ooMsg)

I guess it is the fprintf(stdout, ooMsg) part that needs to be changed ?

EDIT : thx for the confirmation, seth.

Last edited by Lone_Wolf (2025-08-26 22:03:39)


Disliking systemd intensely, but not satisfied with alternatives so focusing on taming systemd.

clean chroot building not flexible enough ?
Try clean chroot manager by graysky

Offline

#6 2025-08-26 21:52:46

seth
Member
From: Don't DM me only for attention
Registered: 2012-09-03
Posts: 69,058

Re: [Solved] fixing format-secuirty errors

seth wrote:
fprintf(stdout, "%s", ooMsg)

Not sure whether the string literal requirement is entirely correct on gcc's part, though.

Online

#7 2025-08-26 22:18:44

loqs
Member
Registered: 2014-03-06
Posts: 18,628

Re: [Solved] fixing format-secuirty errors

seth wrote:

Not sure whether the string literal requirement is entirely correct on gcc's part, though.

ooMsg is s which is char s[2048]; so it should be a string literal?

Offline

#8 2025-08-26 23:31:57

seth
Member
From: Don't DM me only for attention
Registered: 2012-09-03
Posts: 69,058

Re: [Solved] fixing format-secuirty errors

"man 3{p} printf" demands a const char*, so any demand for a string literal here is dubious tbw. (but apparently gcc seeks to validate the patter, variadic parameters) but while every string literal is a char[] not every char[] is a string literal. Technically the latter needs to be const and in practice, it's only "foo"
Notably char s[2048] can go out of context any time.
See eg. https://stackoverflow.com/questions/127 … char-array for speculation and discussions and explanations, but gcc obviously reads "string literal" as "letters between quotes" here.

Online

#9 2025-08-27 00:29:51

loqs
Member
Registered: 2014-03-06
Posts: 18,628

Re: [Solved] fixing format-secuirty errors

Replace `fprintf` with `fwrite` as s is already a formatted string and avoid the issue?

fwrite(ooMsg, sizeof(ooMsg), 1, stdout);

Offline

#10 2025-08-27 06:05:23

dimich
Member
From: Kharkiv, Ukraine
Registered: 2009-11-03
Posts: 414

Re: [Solved] fixing format-secuirty errors

loqs wrote:

Replace `fprintf` with `fwrite` as s is already a formatted string and avoid the issue?

fwrite(ooMsg, sizeof(ooMsg), 1, stdout);

Depending on macro substitution context (whether ooMsg array decay to a pointer to first element or not) this will write either entire buffer with nul terminator and subsequent garbage, or sizeof(pointer) bytes.

This should work:

fwrite(ooMsg, strlen(ooMsg), 1, stdout);

However, printf() with constant format string seems better:

#define OO_SHOW_MSG(ooMsg, ooTitle, ooFlags)	fprintf(stdout, "%s", ooMsg)

Last edited by dimich (2025-08-27 06:06:12)

Offline

#11 2025-08-27 12:04:35

lmn
Member
Registered: 2021-05-09
Posts: 86
Website

Re: [Solved] fixing format-secuirty errors

loqs wrote:

I think you have s and processName mixed up.  s is the output from snprintf https://github.com/OoliteProject/oolite … ain.m#L150 so is already length checked and null terminated. From that it could be argued to just ignore the warning for this specific instance.

I don't think that I have them mixed up (correct me if I'm wrong).

What I mainly was getting at is that the process name could contain a format specifier. Usually the process name is derived from the program file name, but can be adjusted at runtime [1].
The problem that could arise if the binary is named something like 'program%s' and that would be passed to fprintf which would then miss an argument and pop a random value of the stack.
This is just a minor bug and not really important but I wanted to point it out.

I agree with the constant '%s' specifier which sidesteps this problem.

[1] https://developer.gnustep.org/APIRefs/B … rocessName

Edit: Highlight solution

Last edited by lmn (2025-08-27 12:18:16)

Offline

#12 2025-08-27 13:14:46

Lone_Wolf
Administrator
From: Netherlands, Europe
Registered: 2005-10-04
Posts: 14,310

Re: [Solved] fixing format-secuirty errors

Using fprintf(stdout, "%s", ooMsg) fixed that warning, next one looks trickier.

src/SDL/MyOpenGLView.m

src/SDL/MyOpenGLView.m: In function ‘-[MyOpenGLView snapShot:]’:
src/SDL/MyOpenGLView.m:1757:25: error: format not a string literal and no format arguments [-Werror=format-security]
 1757 |                         OOLog(@"screenshotHDR", @"Unrecognized HDR file format requested, defaulting to "SNAPSHOTHDR_EXTENSION_DEFAULT);
      |                         ^

OOLog is used multiple times in this file, defined at lines 94 to 106

#if OOLITE_LINUX
	SDL_SysWMinfo  dpyInfo;
	SDL_VERSION(&dpyInfo.version);
	if(SDL_GetWMInfo(&dpyInfo))
   	{
		nativeDisplayWidth = DisplayWidth(dpyInfo.info.x11.display, 0);
		nativeDisplayHeight = DisplayHeight(dpyInfo.info.x11.display, 0);
		OOLog(@"display.mode.list.native", @"X11 native resolution detected: %d x %d", nativeDisplayWidth, nativeDisplayHeight);
	}
	else
	{
		OOLog(@"display.mode.list.native.failed", @"%@", @"SDL_GetWMInfo failed, defaulting to 1024x768 for native size");
	}

Last edited by Lone_Wolf (2025-08-27 13:15:45)


Disliking systemd intensely, but not satisfied with alternatives so focusing on taming systemd.

clean chroot building not flexible enough ?
Try clean chroot manager by graysky

Offline

#13 2025-08-27 13:38:07

loqs
Member
Registered: 2014-03-06
Posts: 18,628

Re: [Solved] fixing format-secuirty errors

https://github.com/OoliteProject/oolite … ew.m#L1757 "Unrecognized HDR file format requested, defaulting to " missing %s at the end.
Edit:
OOLog can already handle format strings https://github.com/OoliteProject/oolite … .h#L88-L91 the previous issue was a special case since OO_SHOW_MSG was only being used in one instance the easier fix was to hardcode the format string to "%s" in OO_SHOW_MSG's definition rather than use `## __VA_ARGS__` and add the format string to the caller.

Last edited by loqs (2025-08-27 13:53:03)

Offline

#14 2025-08-27 13:56:03

Lone_Wolf
Administrator
From: Netherlands, Europe
Registered: 2005-10-04
Posts: 14,310

Re: [Solved] fixing format-secuirty errors

When adding ,%s at the end a missing expression error came.

Turns out the problem is a missing comma in "SNAPSHOTHDR_EXTENSION_DEFAULT .

No other format security warnings in the sourcecode, going to create an Issue with oolite to get the code fixed.

Thanks for the help.


Disliking systemd intensely, but not satisfied with alternatives so focusing on taming systemd.

clean chroot building not flexible enough ?
Try clean chroot manager by graysky

Offline

#15 2025-08-27 14:07:15

seth
Member
From: Don't DM me only for attention
Registered: 2012-09-03
Posts: 69,058

Re: [Solved] fixing format-secuirty errors

SNAPSHOTHDR_EXTENSION_DEFAULT is defined as  SNAPSHOTHDR_EXTENSION_EXR is defined as @".exr"
The concerned line constructs a string literal using the preprocessor, but the format check apparently doesn't understand that.

OOLog(@"screenshotHDR", @"Unrecognized HDR file format requested, defaulting to ", SNAPSHOTHDR_EXTENSION_DEFAULT);

should still fail but

OOLog(@"screenshotHDR", @"Unrecognized HDR file format requested, defaulting to %s", SNAPSHOTHDR_EXTENSION_DEFAULT);

will work by altering the signature and turning the preprocessor string into the parameter for the "%s" pattern.

Online

#16 2025-08-27 14:26:00

loqs
Member
Registered: 2014-03-06
Posts: 18,628

Re: [Solved] fixing format-secuirty errors

Limitation in gcc that it can detect SNAPSHOTHDR_EXTENSION_DEFAULT is a fixed string so does not need a format string in this case:

OOLog(@"screenshotHDR", @"Unrecognized HDR file format requested, defaulting to ", SNAPSHOTHDR_EXTENSION_DEFAULT);

but does not detect the string concatenation in the original?

Offline

#17 2025-08-27 14:32:56

seth
Member
From: Don't DM me only for attention
Registered: 2012-09-03
Posts: 69,058

Re: [Solved] fixing format-secuirty errors

Is suspect lone_wolf arrived at

OOLog(@"screenshotHDR", @"Unrecognized HDR file format requested, defaulting to %s", SNAPSHOTHDR_EXTENSION_DEFAULT);

which should™ side-step these issues.
Eg.

printf("foo", "bar")

will (correctly) trigger a format error while

printf("foo %s", "bar")

is fine.

Online

#18 2025-08-28 08:24:29

Lone_Wolf
Administrator
From: Netherlands, Europe
Registered: 2005-10-04
Posts: 14,310

Re: [Solved] fixing format-secuirty errors

Issue opened and ooliye devs created a PR 526 for it.

One of them added a comment I think many will agree with.


Disliking systemd intensely, but not satisfied with alternatives so focusing on taming systemd.

clean chroot building not flexible enough ?
Try clean chroot manager by graysky

Offline

#19 2025-08-28 08:34:27

seth
Member
From: Don't DM me only for attention
Registered: 2012-09-03
Posts: 69,058

Re: [Solved] fixing format-secuirty errors

Did %s w/ the NSString actually pass?

Online

#20 2025-08-28 09:23:45

Lone_Wolf
Administrator
From: Netherlands, Europe
Registered: 2005-10-04
Posts: 14,310

Re: [Solved] fixing format-secuirty errors

Yes, gcc accepted either %s and %@ . The format-security check seems to be not very thorough .


Disliking systemd intensely, but not satisfied with alternatives so focusing on taming systemd.

clean chroot building not flexible enough ?
Try clean chroot manager by graysky

Offline

#21 2025-08-28 09:28:07

seth
Member
From: Don't DM me only for attention
Registered: 2012-09-03
Posts: 69,058

Re: [Solved] fixing format-secuirty errors

Well, it thoroughly breaks compilation lol

Online

Board footer

Powered by FluxBB