[hackers] [sbase] uname: check that no operands are specified || Mattias Andrée
commit d8656b0189220e73d3a6cd4ebeaec5091824aef1
Author: Mattias Andrée
AuthorDate: Wed Jul 11 20:53:17 2018 +0200
Commit: Michael Forney
CommitDate: Mon Sep 24 13:04:06 2018 -0700
uname: check that no operands are specified
Signed-off-by: Mattias Andrée
diff --git a/uname.c b/uname.c
index dfc979e..122c172 100644
--- a/uname.c
+++ b/uname.c
@@ -40,6 +40,9 @@ main(int argc, char *argv[])
usage();
} ARGEND
+ if (argc)
+ usage();
+
if (uname() < 0)
eprintf("uname:");
[hackers] [sbase] find: Make -H and -L flag handling clearer || Michael Forney
commit 48d04ae446b39c0e230ec4e8c38d25865fb662db
Author: Michael Forney
AuthorDate: Tue Sep 25 17:57:51 2018 -0700
Commit: Michael Forney
CommitDate: Tue Sep 25 19:16:19 2018 -0700
find: Make -H and -L flag handling clearer
diff --git a/find.c b/find.c
index e095015..03889a8 100644
--- a/find.c
+++ b/find.c
@@ -1023,9 +1023,16 @@ main(int argc, char **argv)
struct tok *t;
ARGBEGIN {
- case 'H': gflags.l = !(gflags.h = 1); break;
- case 'L': gflags.h = !(gflags.l = 1); break;
- default : usage();
+ case 'H':
+ gflags.h = 1;
+ gflags.l = 0;
+ break;
+ case 'L':
+ gflags.l = 1;
+ gflags.h = 0;
+ break;
+ default:
+ usage();
} ARGEND
paths = argv;
Re: [hackers] [PATCH] [sbase] ls: allow listing contents of directories with +r-x
On 2018-07-08, David Phillips wrote:
> As a side note to this patch: it's been sitting in my queue for
> the better part of a year while I internally debated the
> untidiness of not being allowed to chdir. It's an unfortunate
> result of implementing this behaviour, but it's behaviour
> that other implementations (namely GNU, but not Solaris) support.
>
> Let me know any feedback or oversights on my part,
> David
Hi David,
Rather than keeping track of the full prefix for `mkent` and `output`,
what do you think about just tracking the directory fd, and using the
*at family of functions to work relative to that directory? I attached
a patch to show what I'm thinking. It looks like this might even
simplify some of the code.
Either way, I think the changes to avoid chdir and to show unknown
entries (stat failure) should be separate commits.
From 7bd0f40e2166788a91f0015cb7386e44f105fad8 Mon Sep 17 00:00:00 2001
From: Michael Forney
Date: Tue, 25 Sep 2018 18:23:47 -0700
Subject: [PATCH] ls: Use *at functions instead of changing working directory
---
ls.c | 52 ++--
1 file changed, 22 insertions(+), 30 deletions(-)
diff --git a/ls.c b/ls.c
index b716aba..7535c0b 100644
--- a/ls.c
+++ b/ls.c
@@ -3,6 +3,7 @@
#include
#include
+#include
#include
#include
#include
@@ -55,18 +56,18 @@ static int first = 1;
static char sort = 0;
static int showdirs;
-static void ls(const char *, const struct entry *, int);
+static void ls(int, const char *, const struct entry *, int);
static void
-mkent(struct entry *ent, char *path, int dostat, int follow)
+mkent(int dirfd, struct entry *ent, char *path, int dostat, int follow)
{
struct stat st;
ent->name = path;
if (!dostat)
return;
- if ((follow ? stat : lstat)(path, ) < 0)
- eprintf("%s %s:", follow ? "stat" : "lstat", path);
+ if (fstatat(dirfd, path, , follow ? 0 : AT_SYMLINK_NOFOLLOW) < 0)
+ eprintf("stat %s:", path);
ent->mode = st.st_mode;
ent->nlink = st.st_nlink;
ent->uid = st.st_uid;
@@ -130,7 +131,7 @@ printname(const char *name)
}
static void
-output(const struct entry *ent)
+output(int dirfd, const struct entry *ent)
{
struct group *gr;
struct passwd *pw;
@@ -208,7 +209,7 @@ output(const struct entry *ent)
printname(ent->name);
fputs(indicator(ent->mode), stdout);
if (S_ISLNK(ent->mode)) {
- if ((len = readlink(ent->name, buf, sizeof(buf) - 1)) < 0)
+ if ((len = readlinkat(dirfd, ent->name, buf, sizeof(buf) - 1)) < 0)
eprintf("readlink %s:", ent->name);
buf[len] = '\0';
printf(" -> %s%s", buf, indicator(ent->tmode));
@@ -239,7 +240,7 @@ entcmp(const void *va, const void *vb)
}
static void
-lsdir(const char *path, const struct entry *dir)
+lsdir(int dirfd, const char *path, const struct entry *dir)
{
DIR *dp;
struct entry *ent, *ents = NULL;
@@ -247,13 +248,12 @@ lsdir(const char *path, const struct entry *dir)
size_t i, n = 0;
char prefix[PATH_MAX];
- if (!(dp = opendir(dir->name))) {
+ dirfd = openat(dirfd, dir->name, O_RDONLY|O_DIRECTORY);
+ if (dirfd < 0 || !(dp = fdopendir(dirfd))) {
ret = 1;
weprintf("opendir %s%s:", path, dir->name);
return;
}
- if (chdir(dir->name) < 0)
- eprintf("chdir %s:", dir->name);
while ((d = readdir(dp))) {
if (d->d_name[0] == '.' && !aflag && !Aflag)
@@ -264,12 +264,10 @@ lsdir(const char *path, const struct entry *dir)
continue;
ents = ereallocarray(ents, ++n, sizeof(*ents));
- mkent([n - 1], estrdup(d->d_name), Fflag || iflag ||
+ mkent(dirfd, [n - 1], estrdup(d->d_name), Fflag || iflag ||
lflag || pflag || Rflag || sort, Lflag);
}
- closedir(dp);
-
if (!Uflag)
qsort(ents, n, sizeof(*ents), entcmp);
@@ -279,7 +277,7 @@ lsdir(const char *path, const struct entry *dir)
puts(":");
}
for (i = 0; i < n; i++)
- output([i]);
+ output(dirfd, [i]);
if (Rflag) {
if (snprintf(prefix, PATH_MAX, "%s%s/", path, dir->name) >=
@@ -294,13 +292,14 @@ lsdir(const char *path, const struct entry *dir)
if (S_ISLNK(ent->mode) && S_ISDIR(ent->tmode) && !Lflag)
continue;
- ls(prefix, ent, 1);
+ ls(dirfd, prefix, ent, 1);
}
}
for (i = 0; i < n; ++i)
free(ents[i].name);
free(ents);
+ closedir(dp);
}
static int
@@ -325,13 +324,12 @@ visit(const struct entry *ent)
}
static void
-ls(const char *path, const struct entry *ent, int listdir)
+ls(int dirfd, const char *path, const struct entry *ent, int listdir)
{
int treeind;
- char cwd[PATH_MAX];
if (!listdir) {
- output(ent);
+ output(dirfd, ent);
} else if (S_ISDIR(ent->mode) ||
(S_ISLNK(ent->mode) && S_ISDIR(ent->tmode))) {
if ((treeind = visit(ent)) < 0) {
@@ -340,19 +338,13 @@ ls(const char *path, const struct entry *ent, int listdir)
return;
}
- if (!getcwd(cwd, PATH_MAX))
- eprintf("getcwd:");
-
if (first)
first = 0;
else
putchar('\n');
- lsdir(path, ent);
+ lsdir(dirfd, path, ent);
tree[treeind].ino = 0;
-
- if
Re: [hackers] [dwm][PATCH] Fix use-after-free on cleanup.
On Mon, 24 Sep 2018 at 21:14, Mario J. Rugiero wrote: > When cleaning up the stack the stack member for the first > monitor wasn't being updated to reflect this, with the following > (possible) consequences: > - An infinite loop. If things wouldn't crash, not updating the > guard of the loop would lead to this. > - Garbage being read and passed to functions. > - A double free on m->stack. How do you came to this weird conclusion? m->stack is detached on both the stack and the list prior to any free'ing or other handling during unmanage. Rejected. -Anselm
Re: [hackers] [dwm][PATCH] Fail zoom on no selection.
On Mon, 24 Sep 2018 at 21:14, Mario J. Rugiero wrote: > Continuing on '!selmon->sel' leads to a NULL pointer dereference. ??? Rejected. -Anselm
[hackers] [ubase][PATCH] passwd: fix crashes when authentication is unnecessary.
From: Mario Rugiero
When running with root or a password for the user is missing,
authentication is bypassed.
However, it is later attempted to compare the new password
against the missing one, causing crypt to crash due to a null
salt.
In the case of a missing password, there's no prior password
to compare to, so the only choice is to avoid the comparison.
In the case of root, reading a password (if present) is possible,
to avoid resetting to the same password. However, it seems benign
to just let it be to avoid more confusion.
Anyway, the fix consists on doing the check only if we got an
old password to begin with.
---
passwd.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/passwd.c b/passwd.c
index 52b70a8..92c59fd 100644
--- a/passwd.c
+++ b/passwd.c
@@ -235,11 +235,13 @@ newpass:
eprintf("getpass:");
if (inpass[0] == '\0')
eprintf("no password supplied\n");
- p = crypt(inpass, prevhash);
- if (!p)
- eprintf("crypt:");
- if (cryptpass1 && strcmp(cryptpass1, p) == 0)
- eprintf("password left unchanged\n");
+ if (prevhash) {
+ p = crypt(inpass, prevhash);
+ if (!p)
+ eprintf("crypt:");
+ if (strcmp(cryptpass1, p) == 0)
+ eprintf("password left unchanged\n");
+ }
gensalt(salt + strlen(salt));
p = crypt(inpass, salt);
if (!p)
--
2.17.1
[hackers] [ubase][PATCH] passwd: fix crashes for unencrypted passwords starting with 'x'.
From: Mario Rugiero
When deciding where the previous hash should come from, is is
assumed that 'x' started strings all mean to look in shadow.
This is probably harmless in practice, since modern Linux still
use only hashes instead of raw passwords.
However, this is more robust, and more importantly, it is more
consistent with the previous check, which explicitly tests for
the string to be "x".
---
passwd.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/passwd.c b/passwd.c
index 92c59fd..53e01e8 100644
--- a/passwd.c
+++ b/passwd.c
@@ -210,7 +210,8 @@ main(int argc, char *argv[])
if (pw->pw_passwd[0] == '\0') {
goto newpass;
}
- if (pw->pw_passwd[0] == 'x')
+ if (pw->pw_passwd[0] == 'x' &&
+ pw->pw_passwd[1] == '\0')
prevhash = spw->sp_pwdp;
else
prevhash = pw->pw_passwd;
--
2.17.1
Re: [hackers] [ubase][PATCH] passwd: fix crashes when authentication is unnecessary.
El mar., 25 sep. 2018 02:18, Quentin Rameau escribió:
> Hey Mario,
>
> > - p = crypt(inpass, prevhash);
> > - if (!p)
> > - eprintf("crypt:");
> > - if (cryptpass1 && strcmp(cryptpass1, p) == 0)
> > - eprintf("password left unchanged\n");
> > + if (cryptpass1) {
>
> ^--- for more clarity, maybe check against prevhash instead?
>
That makes sense.
>
> > + p = crypt(inpass, prevhash);
> > + if (!p)
> > + eprintf("crypt:");
> > + if (strcmp(cryptpass1, p) == 0)
> > + eprintf("password left unchanged\n");
> > + }
>
>
Re: [hackers] [ubase][PATCH] passwd: fix crashes for unencrypted passwords starting with 'x'.
El mar., 25 sep. 2018 02:03, Quentin Rameau escribió: > Hello Mario, > > > - if (pw->pw_passwd[0] == 'x') > > + if (pw->pw_passwd[0] == 'x' && > > + pw->pw_passwd[0] == '\0') > > Did you mean “pw->pw_passwd[1] == '\0')”? > > Yes. I'll send a fix later.
Re: [hackers] [st][patch] Increase the buffer size for escape sequences
Hi Roberto, > If we go to increase that size, I would go to use dynamic memory. Having > an array of 1MB statically allocated is a crazy idea Yes it is! > (and it is not C99 compliant, where the maximun allocated size is 128K). That's actually 64k (“at least […] — 65535 bytes in an object (in a hosted environment only)”) ;)
Re: [hackers] [st][patch] Increase the buffer size for escape sequences
On Tue, Sep 25, 2018 at 10:05 AM Roberto E. Vargas Caballero wrote: > On Mon, Sep 24, 2018 at 05:45:29PM -0700, Eric Pruitt wrote: > > I agree that the current buffer is too small. I'm pretty sure I've run > > into this problem myself with Vim and Bash, but I hadn't gotten around > > to digging into the problem. > > If we go to increase that size, I would go to use dynamic memory. Having > an array of 1MB statically allocated is a crazy idea (and it is not > C99 compliant, where the maximun allocated size is 128K). On my machine, st uses 11MB up to 15MB of memory currently. If we just add 1MB of statically-allocated space it would blow up that size by 9% in some cases so I am also in favor of using dynamic memory there.
Re: [hackers] [st][patch] Increase the buffer size for escape sequences
On Mon, Sep 24, 2018 at 05:45:29PM -0700, Eric Pruitt wrote: > I agree that the current buffer is too small. I'm pretty sure I've run > into this problem myself with Vim and Bash, but I hadn't gotten around > to digging into the problem. If we go to increase that size, I would go to use dynamic memory. Having an array of 1MB statically allocated is a crazy idea (and it is not C99 compliant, where the maximun allocated size is 128K).
Re: [hackers] [PATCH][sbase] find: fix flag setting
On Mon, Sep 24, 2018 at 10:03 PM Michael Forney wrote: > On 7/8/18, Silvan Jegen wrote: > > Heyho > > > > Found this when running smatch on sbase. > > The current code is correct. -H should turn on gflags.h, and turn off > gflags.l. POSIX says each flag should override the other. I agree that > the current code is a little confusing. Yes, you are right. Smatch and me got confused. In my case it was probably because I didn't know that assignments in C return the assigned value. > It would probably be clearer as > > case 'H': gflags.h = 1; gflags.l = 0; break; > case 'L': gflags.h = 0; gflags.l = 1; break; I also think this would be clearer and shorter. The current code is correct though so maybe it's not worth to send a patch. Thanks for having a look! Cheers, Silvan
