#16 build error (stringop-truncation)

[Leandro Nini]

  In file included from /usr/include/string.h:495,
                   from ../../config-bot.h:35,
                   from ../../config.h:167,
                   from log.c:9:
  In function ‘strncpy’,
      inlined from ‘porg_get_absolute_path’ at log.c:110:3:
  /usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ output may be truncated copying 4095 bytes from a string of length 4095 [-Werror=stringop-truncation]
    106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
        |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  In function ‘strncpy’,
      inlined from ‘porg_get_absolute_path’ at log.c:104:3:
  /usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ output may be truncated copying 4095 bytes from a string of length 4095 [-Werror=stringop-truncation]
    106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
        |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  cc1: all warnings being treated as errors

[Ookaze]

This bug was corrected in a previous ticket in 2016.
The patch was wrong though, the pragma added introduced a bug that prevents logging of some files. I finally tackled the problem yesterday. Here is my patch from 0.10 for these bugs.
I've made a tarball for my 0.10.2 version as the autotools are outdated too in 0.10. I think the tool is ripe for a minor version release.

[Leandro Nini]

Ignoring the warnings looks more like an hack than a fix.
I'd better null terminate the string after strncpy to prevent possible buffer overflows:

--- a/lib/porg-log/log.c
+++ b/lib/porg-log/log.c
@@ -102,12 +102,14 @@ static void porg_get_absolute_path(int fd, const char* path, char* abs_path)
    /* relative to CWD */
    else if (fd < 0) {
        strncpy(abs_path, cwd, PORG_BUFSIZE - 1);
+       abs_path[PORG_BUFSIZE - 1] = 0;
        strncat(abs_path, "/", PORG_BUFSIZE - strlen(abs_path) - 1);
        strncat(abs_path, path, PORG_BUFSIZE - strlen(abs_path) - 1);
    }
    /* relative to directory fd */
    else if (fchdir(fd) == 0 && getcwd(aux, PORG_BUFSIZE) && chdir(cwd) == 0) {
        strncpy(abs_path, aux, PORG_BUFSIZE - 1);
+       abs_path[PORG_BUFSIZE - 1] = 0;
        strncat(abs_path, "/", PORG_BUFSIZE - strlen(abs_path) - 1);
        strncat(abs_path, path, PORG_BUFSIZE - strlen(abs_path) - 1);
    }

[Ookaze]

You're right, iirc this was David's patch back in 2016. I haven't done anything but correct the bug that was very annoying and bit me hard recently with the release of coreutils 9.1 where the bug made porg not work anymore.
Now that you talk about buffer overflows, it reminds me that even if at functions are more secure, we still have no control over the actual directory name, and that I have to check that I didn't forget to add another fix because the code sometimes puts directory names starting with '//' instead of '/', which still works but is incorrect. This kind of flawed but working code can lead to exploitation (in doubt I've included it). The second strlen is costly, I don't know if GCC will optimize it, I'd better not do it manually as there's a time window of exploitation between the first strlen and the second.
I agree with you on these problems, but I think the best course of action is asking David.
Meanwhile, I will do exactly as you said to mitigate any risk.
I've had porg crash (because of asserts) when reading its own files after installing some node tool which put me in alert mode recently, I think it all will be fixed with these patches.
Meanwhile, I've corrected these.

[David Ricart]

Hello everybody.

I am David, the creator and maintainer of porg.

I wanted you to know that my life has led me far from computer
programming lately, so I don't have the time and energy to keep
maintaining this project anymore.

I am willing to listen to any candidate to take it over.

Cheers,

~David.

2022-05-26 16:44 GMT+02:00, Ookaze ookaze0@users.sourceforge.net:

You're right, iirc this was David's patch back in 2016. I haven't done
anything but correct the bug that was very annoying and bit me hard recently
with the release of coreutils 9.1 where the bug made porg not work anymore.
Now that you talk about buffer overflows, it reminds me that even if at
functions are more secure, we still have no control over the actual
directory name, and that I have to check that I didn't forget to add another
fix because the code sometimes puts directory names starting with '//'
instead of '/', which still works but is incorrect. This kind of flawed but
working code can lead to exploitation (in doubt I've included it). The
second strlen is costly, I don't know if GCC will optimize it, I'd better
not do it manually as there's a time window of exploitation between the
first strlen and the second.
I agree with you on these problems, but I think the best course of action is
asking David.
Meanwhile, I will do exactly as you said to mitigate any risk.
I've had porg crash (because of asserts) when reading its own files after
installing some node tool which put me in alert mode recently, I think it
all will be fixed with these patches.
Meanwhile, I've corrected these.

Attachments:

-
porg-0.10-log.c.patch
(2.3 kB; text/x-patch)

---

[tickets:#16] build error (stringop-truncation)

Status: open
Milestone: 1.0
Created: Fri Apr 22, 2022 09:12 AM UTC by Leandro Nini
Last Updated: Thu May 26, 2022 05:57 AM UTC
Owner: nobody

~~~
In file included from /usr/include/string.h:495,
from ../../config-bot.h:35,
from ../../config.h:167,
from log.c:9:
In function ‘strncpy’,
inlined from ‘porg_get_absolute_path’ at log.c:110:3:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error:
‘__builtin_strncpy’ output may be truncated copying 4095 bytes from a string
of length 4095 [-Werror=stringop-truncation]
106 | return __builtinstrncpychk (dest, __src, __len, __bos
(__dest));
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ‘strncpy’,
inlined from ‘porg_get_absolute_path’ at log.c:104:3:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error:
‘__builtin_strncpy’ output may be truncated copying 4095 bytes from a string
of length 4095 [-Werror=stringop-truncation]
106 | return __builtinstrncpychk (dest, __src, __len, __bos
(__dest));
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
~~~

---

Sent from sourceforge.net because you indicated interest in
https://sourceforge.net/p/porg/tickets/16/

To unsubscribe from further messages, please visit
https://sourceforge.net/auth/subscriptions/

[Ookaze]

Hi David,

I've used porg constantly since paco as I compile my own OS for all my servers and clients for 20+ y now. I can take back the development, though I'm not familiar with the sourceforge process yet. If it's allowed, I would like to put the code on github too.
I won't be especially active after the initial cleanup, but if noone else wants to take the dev back, I'm ready to manage this.

[David Ricart]

Hello "Ookaze" (your name?).

Yes, I remember you being quite active in porg / paco issues since
long time ago. So in principle I am willing to consider your kind
offer to take over the project.

But you'd have to meet certain conditions, like keeping it free,
simple, and give credit to the original author. Also, since I am no
longer familiar with the sourceforge process, you'd have to make the
transition to other code hosting platforms yourself.

Also, you'd have to establish some contact with the maintainers of the
debian and ubuntu porg packages, to let them know the new status of
the project.

Do you find it fair enough?

Cheers,

~David Ricart.

2022-06-07 21:04 GMT+02:00, Ookaze ookaze0@users.sourceforge.net:

Hi David,

I've used porg constantly since paco as I compile my own OS for all my
servers and clients for 20+ y now. I can take back the development, though
I'm not familiar with the sourceforge process yet. If it's allowed, I would
like to put the code on github too.
I won't be especially active after the initial cleanup, but if noone else
wants to take the dev back, I'm ready to manage this.

---

[tickets:#16] build error (stringop-truncation)

Status: open
Milestone: 1.0
Created: Fri Apr 22, 2022 09:12 AM UTC by Leandro Nini
Last Updated: Thu May 26, 2022 02:44 PM UTC
Owner: nobody

~~~
In file included from /usr/include/string.h:495,
from ../../config-bot.h:35,
from ../../config.h:167,
from log.c:9:
In function ‘strncpy’,
inlined from ‘porg_get_absolute_path’ at log.c:110:3:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error:
‘__builtin_strncpy’ output may be truncated copying 4095 bytes from a string
of length 4095 [-Werror=stringop-truncation]
106 | return __builtinstrncpychk (dest, __src, __len, __bos
(__dest));
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ‘strncpy’,
inlined from ‘porg_get_absolute_path’ at log.c:104:3:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error:
‘__builtin_strncpy’ output may be truncated copying 4095 bytes from a string
of length 4095 [-Werror=stringop-truncation]
106 | return __builtinstrncpychk (dest, __src, __len, __bos
(__dest));
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
~~~

---

Sent from sourceforge.net because you indicated interest in
https://sourceforge.net/p/porg/tickets/16/

To unsubscribe from further messages, please visit
https://sourceforge.net/auth/subscriptions/

[Leandro Nini]

Still any interest in reviving the project?
I have my own clone on github with a bunch of fixes, but it would be good to have an official release.

[Jean-Michel Pollion]

I made a proposal to David more than 1 year ago but I made little progress, apart from creating a fork too. I need to streamline several processes before seriously taking the project back, I'm still interested but if someone else wants to revive this project, that's not a problem for me.