ucspi-unix: a couple of strtoul() fixes

Peter Pentchev <[email protected]> Thu, 12 Dec 2024 23:10:06 +0200
Newsgroups gmane.comp.sysutils.bgware
Message-ID <[email protected]>
--VBvSlIjVMo+0PqqD
Content-Type: multipart/mixed; boundary="jU2xl0RHh1ijLsMp"
Content-Disposition: inline


--jU2xl0RHh1ijLsMp
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi,

Thanks a lot for writing and maintaining ucspi-unix!

What do you think about the attached couple of patches that deal with
the way strtoul(3) is invoked to parse various command-line options?

- the first patch applies to any POSIX system ucspi-unix runs on, and
  makes unixserver report an error if a number is too large,
  e.g. `unixserver -u 9999999999999999999999` would not report an error,
  yet it would not set `opt_uid` at all
- the second patch makes sure that the number parsed by strtoul(3) can
  indeed fit into an unsigned integer variable; this only applies to
  systems where sizeof(unsigned int) !=3D sizeof(unsigned long), which is
  not really that common, but still it might happen
- the third patch fixes the compilation on systems where uid_t, gid_t, and
  mode_t are unsigned short values instead of unsigned int ones,
  e.g. Linux on the 32-bit i386 platform

Thanks in advance, and keep up the great work!

G'luck,
Peter

--=20
Peter Pentchev  [email protected] [email protected] [email protected]
PGP key:        https://www.ringlet.net/roam/roam.key.asc
Key fingerprint 2EE7 A7A5 17FC 124C F115  C354 651E EFB0 2527 DF13

--jU2xl0RHh1ijLsMp
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="strtoul-errno.patch"
Content-Transfer-Encoding: quoted-printable

Description: Catch overflow errors when converting strings to numbers
 Passing e.g. `-u 99999999999` would not change the variable at all, yet
 not be reported as an error.
Forwarded: no
Author: Peter Pentchev <[email protected]>
Last-Update: 2024-12-12

--- a/unixserver.c
+++ b/unixserver.c
@@ -1,3 +1,4 @@
+#include <errno.h>
 #include <signal.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -102,8 +103,9 @@
 {
   char* ptr;
   if (!str) return 0;
+  errno =3D 0;
   *out =3D strtoul(str, &ptr, base);
-  return (*ptr =3D=3D 0);
+  return (*ptr =3D=3D 0) && (errno =3D=3D 0);
 }
=20
 static void use_uid(const char* str)

--jU2xl0RHh1ijLsMp
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="strtoul-max.patch"
Content-Transfer-Encoding: quoted-printable

Description: Make sure the strtoul() value is a valid unsigned int
Forwarded: no
Author: Peter Pentchev <[email protected]>
Last-Update: 2024-12-12

--- a/unixserver.c
+++ b/unixserver.c
@@ -1,4 +1,5 @@
 #include <errno.h>
+#include <limits.h>
 #include <signal.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -99,7 +100,7 @@
   exit(1);
 }
=20
-static int parseu(const char* str, unsigned* out, int base)
+static int parseul(const char* str, unsigned long* out, int base)
 {
   char* ptr;
   if (!str) return 0;
@@ -108,6 +109,14 @@
   return (*ptr =3D=3D 0) && (errno =3D=3D 0);
 }
=20
+static int parseu(const char* str, unsigned* out, int base)
+{
+  unsigned long ulval;
+  if (!parseul(str, &ulval, base) || ulval > UINT_MAX) return 0;
+  *out =3D (unsigned)ulval;
+  return 1;
+}
+
 static void use_uid(const char* str)
 {
   if (!str) usage("UID not found in environment.");

--jU2xl0RHh1ijLsMp
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="strtoul-int-gid.patch"
Content-Transfer-Encoding: quoted-printable

Description: Use `unsigned short` for `uid_t`, `gid_t`, and `mode_t` if nee=
ded
Forwarded: no
Author: Peter Pentchev <[email protected]>
Last-Update: 2024-12-12

--- a/unixserver.c
+++ b/unixserver.c
@@ -9,6 +9,7 @@
 #include <sys/un.h>
 #include <sys/wait.h>
 #include <unistd.h>
+#include "hasintgid.h"
 #include "haswaitp.h"
=20
 extern void setup_env(int, const char*);
@@ -117,28 +118,48 @@
   return 1;
 }
=20
+#if defined(HASINTGID)
+static int parsegid(const char *str, gid_t *out, int base)
+{
+	return parseu(str, out, base);
+}
+#else
+static int parseus(const char* str, unsigned short* out, int base)
+{
+  unsigned long ulval;
+  if (!parseul(str, &ulval, base) || ulval > USHRT_MAX) return 0;
+  *out =3D (unsigned short)ulval;
+  return 1;
+}
+
+static int parsegid(const char *str, gid_t *out, int base)
+{
+	return parseus(str, out, base);
+}
+#endif
+
 static void use_uid(const char* str)
 {
   if (!str) usage("UID not found in environment.");
-  if (!parseu(str, &opt_uid, 10)) usage("Invalid UID number");
+  if (!parsegid(str, &opt_uid, 10)) usage("Invalid UID number");
 }
=20
 static void use_gid(const char* str)
 {
   if (!str) usage("GID not found in environment.");
-  if (!parseu(str, &opt_gid, 10)) usage("Invalid GID number");
+  if (!parsegid(str, &opt_gid, 10)) usage("Invalid GID number");
 }
=20
 static void use_socket_uid(const char* str)
 {
   if (!str) usage("Socket UID not found in environment.");
-  if (!parseu(str, &opt_socket_uid, 10)) usage("Invalid socket UID number"=
);
+  if (!parsegid(str, &opt_socket_uid, 10)) usage("Invalid socket UID numbe=
r");
 }
=20
 static void use_socket_gid(const char* str)
 {
   if (!str) usage("Socket GID not found in environment.");
-  if (!parseu(str, &opt_socket_gid, 10)) usage("Invalid socket GID number"=
);
+  if (!parsegid(str, &opt_socket_gid, 10)) usage("Invalid socket GID numbe=
r");
 }
=20
 void parse_options(int argc, char* argv[])
@@ -169,10 +190,10 @@
       use_socket_gid(getenv("SOCKET_GID"));
       break;
     case 'p':
-      if (!parseu(optarg, &opt_perms, 8)) usage("Invalid permissions value=
=2E");
+      if (!parsegid(optarg, &opt_perms, 8)) usage("Invalid permissions val=
ue.");
       break;
     case 'm':
-      if (!parseu(optarg, &opt_umask, 8)) usage("Invalid mask value.");
+      if (!parsegid(optarg, &opt_umask, 8)) usage("Invalid mask value.");
       break;
     case 'b':
       if (!parseu(optarg, &opt_backlog, 10)) usage("Invalid backlog count.=
");
--- a/Makefile
+++ b/Makefile
@@ -31,6 +31,10 @@
 	( ( ./compile trypeercred.c && ./load trypeercred; ) && cat haspeercred.h=
1 || cat haspeercred.h0 ) > $@
 	@rm -f trypeercred.o trypeercred
=20
+hasintgid.h: compile load hasintgid.h0 hasintgid.h1 tryintgid.c
+	( ( ./compile tryintgid.c && ./load tryintgid; ) && cat hasintgid.h1 || c=
at hasintgid.h0 ) > $@
+	@rm -f tryintgid.o tryintgid
+
 haswaitp.h: compile load haswaitp.h0 haswaitp.h1 trywaitp.c
 	( ( ./compile trywaitp.c && ./load trywaitp; ) && cat haswaitp.h1 || cat =
haswaitp.h0 ) > $@
 	@rm -f trywaitp.o trywaitp
@@ -79,7 +83,7 @@
 unixserver.1.html: unixserver.1
 	man --html=3D'cat %s' ./unixserver.1 > unixserver.1.html
=20
-unixserver.o: compile unixserver.c haswaitp.h
+unixserver.o: compile unixserver.c hasintgid.h haswaitp.h
 	./compile unixserver.c
=20
 utoa.o: compile utoa.c
--- a/TARGETS
+++ b/TARGETS
@@ -4,6 +4,7 @@
 compile
 docs
 env.o
+hasintgid.h
 haspeercred.h
 haswaitp.h
 install
--- /dev/null
+++ b/hasintgid.h0
@@ -0,0 +1 @@
+/* sysdep: -intgid */
--- /dev/null
+++ b/hasintgid.h1
@@ -0,0 +1,2 @@
+/* sysdep: +intgid */
+#define HASINTGID
--- /dev/null
+++ b/tryintgid.c
@@ -0,0 +1,14 @@
+#include <sys/types.h>
+#include <stdio.h>
+
+void output(unsigned int *value)
+{
+	printf("%u\n", *value);
+}
+
+int main()
+{
+  gid_t gid =3D 3;
+  output(&gid);
+  return 0;
+}

--jU2xl0RHh1ijLsMp--

--VBvSlIjVMo+0PqqD
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEELuenpRf8EkzxFcNUZR7vsCUn3xMFAmdbUSkACgkQZR7vsCUn
3xN9lhAAj4Eb0QRfT9kJ+ELjgg3GjScRAwV+vKfe6ZTW3b8tcUuUz0tXwuRzMUMw
olYWpfmbnJq14nDUy8ueJ9dyeFgGlnIrfzOwXM4EQhCCCaYaqiwwAoeVBSAlgJAn
ZKQk3YIY2UYHuQcJZlNq9apT281caOmFqR30JWImKZxIN3toNrSKCmAbbAp5xN80
TSpXXF2AVtryYRBUfdS29BOFSWdCT4eF+0cF1MEhwZY+W1GbQeIlR6yhon/VyhBU
xLZwzecgIqETCSsiPpcf9MWGJ7xplZQqY4w6Cr7tzHclhMWKGKrxnHn7/svS5e/m
3nOH2+5p/nIzRwQj4me4mLJd+98lYi3PBIKvTf1XGGGeKCs93kMrZLdbR0P7RR0p
ANlFWCYR4SsfOvfcQKZ0z7L6Y0/EIofbYOjl7fUejfk9vMv8lHlZ43Qosu+gkY4M
gvFVTK9Sl+uN38ZTejnaGA25fKkdQPULAaAvOKDmxzRdu3RedIQaq7wIMh67Xtkv
Eo5zk8hiM4wey/gC+lBdQJEnNWMsAnxAwbcYYdgp435zWOzLGFrse5fvj7e3ufsx
eDA6d+jvOL8jQwaAkkKFxp/Mwv9GFL1uZGTq9NGXqljpQ5M5vy1wavjVBuUlz+DN
2ivrR5ehYLuY/JO/PLqMqcdVdCuq21hbh3ybKZfVNnzhjBlvHG4=
=xueC
-----END PGP SIGNATURE-----

--VBvSlIjVMo+0PqqD--