Re: [hackers] [dwm][PATCH] move config data to read-only sections

From: Anselm R Garbe <garbeam_AT_gmail.com>
Date: Thu, 7 Sep 2017 09:33:27 +0200

Hi Jo,

On 6 September 2017 at 21:04, <Joachim.Henke_AT_t-systems.com> wrote:
>> From: Anselm R Garbe [garbeam_AT_gmail.com]
>> Sent: Wednesday, September 6, 2017 7:38 PM
>> To: hackers mail list
>> Subject: Re: [hackers] [dwm][PATCH] move config data to read-only sections

>> Are you suggesting to typedef const char *const coChar; or something
>> similar and then using coChar in those places? I would still prefer
>> the old way for readability reasons. There is no real impact either
>> imho.

> In information security, it is good practice to use the principle of least privilege. Setting data 'const' could not only help to spot future programming errors, but essentially keeps this data in a read-only memory page during run-time! While I agree that the C syntax for read-only pointers is not that readable, I don't understand, that you're seriously trading this against security.

I don't really see this as a tradeoff against security in the
particular case of how dwm uses those declarations in its code base.
The config.h declarations are only used as inputs for the setup()
stage and for key handler arguments. The vanilla dwm code isn't
manipulating those values and also not processing other inputs during
runtime, except for certain X atoms, that are unrelated to the static
declarations.

I only see your suggestion as advantage for patch creators to avoid
unwanted side-effects. However, the potential threat vector of patches
is typically in runtime behaviour and not so much in manipulating any
vanilla dwm declarations -- and the user applying patches should
always carefully check what the patch actually does and not blindly
trust a patch implementation, as it is not an official part of the dwm
source.

> Regarding 'typedef' I had something like the following in mind (you might want to use another name):
>
> typedef const char *stringPtr;
>
> Then a declaration could look like this:
>
> static const stringPtr termcmd[] = { "st", NULL };
>
> Maybe not the most beautiful, but IMHO it makes clear that this is an array of constant string pointers. – Both, gcc and clang, then properly put this in read-only ELF sections.

I will think about this. Perhaps we could introduce something similar,
as this looks acceptable to some extend. Perhaps StrPtr could be used
as typename for this.

Best regards,
Anselm
Received on Thu Sep 07 2017 - 09:33:27 CEST