Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issues found by CodeQL #854

Merged
merged 10 commits into from
Sep 2, 2021
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
[#811](https://github.com/greenbone/openvas/pull/811)

### Fixed
- Use fchmod to change file permission instead of on open to prevent race conditions [854](https://github.com/greenbone/openvas-scanner/pull/854)
- Several minor potential security risks in different files, spotted by Code QL [854](https://github.com/greenbone/openvas-scanner/pull/854)

### Removed
- Remove handling of source_iface related preferences. [#730](https://github.com/greenbone/openvas/pull/730)
Expand Down
6 changes: 5 additions & 1 deletion nasl/md4.c
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,11 @@ mdfour64_ntlmssp (uint32 *M)
B &= 0xFFFFFFFF;
C &= 0xFFFFFFFF;
D &= 0xFFFFFFFF;
memset (X, '\0', sizeof (X));

for (size_t i = 0; i < sizeof (X); i++)
{
*(volatile char *) (X + i) = '\0';
}
}

static void
Expand Down
52 changes: 12 additions & 40 deletions nasl/nasl_cmd_exec.c
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ nasl_file_open (lex_ctxt *lexic)
{
tree_cell *retc;
char *fname, *mode;
struct stat lstat_info, fstat_info;
struct stat fstat_info;
int fd;
int imode = O_RDONLY;

Expand Down Expand Up @@ -458,48 +458,20 @@ nasl_file_open (lex_ctxt *lexic)
else if (strcmp (mode, "a+") == 0)
imode = O_RDWR | O_APPEND | O_CREAT;

if (lstat (fname, &lstat_info) == -1)
fd = open (fname, imode, 0600);
if (fd < 0)
{
if (errno != ENOENT)
{
nasl_perror (lexic, "file_open: %s: %s\n", fname, strerror (errno));
return NULL;
}
fd = open (fname, imode, 0600);
if (fd < 0)
{
nasl_perror (lexic, "file_open: %s: %s\n", fname, strerror (errno));
return NULL;
}
nasl_perror (lexic, "file_open: %s: possible symlink attack!?! %s\n",
fname, strerror (errno));
return NULL;
}
else

if (fstat (fd, &fstat_info) == -1)
{
fd = open (fname, imode, 0600);
if (fd < 0)
{
nasl_perror (lexic, "file_open: %s: possible symlink attack!?! %s\n",
fname, strerror (errno));
return NULL;
}
if (fstat (fd, &fstat_info) == -1)
{
close (fd);
nasl_perror (lexic, "fread: %s: possible symlink attack!?! %s\n",
fname, strerror (errno));
return NULL;
}
else
{
if (lstat_info.st_mode != fstat_info.st_mode
|| lstat_info.st_ino != fstat_info.st_ino
|| lstat_info.st_dev != fstat_info.st_dev)
{
close (fd);
nasl_perror (lexic, "fread: %s: possible symlink attack!?!\n",
fname);
return NULL;
}
}
close (fd);
nasl_perror (lexic, "fread: %s: possible symlink attack!?! %s\n", fname,
strerror (errno));
return NULL;
}

retc = alloc_typed_cell (CONST_INT);
Expand Down
8 changes: 4 additions & 4 deletions nasl/nasl_isotime.c
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,12 @@ epoch2isotime (my_isotime_t timebuf, time_t atime)
*timebuf = 0;
else
{
struct tm *tp;
struct tm tp;

tp = gmtime (&atime);
gmtime_r (&atime, &tp);
if (snprintf (timebuf, ISOTIME_SIZE, "%04d%02d%02dT%02d%02d%02d",
1900 + tp->tm_year, tp->tm_mon + 1, tp->tm_mday,
tp->tm_hour, tp->tm_min, tp->tm_sec)
1900 + tp.tm_year, tp.tm_mon + 1, tp.tm_mday, tp.tm_hour,
tp.tm_min, tp.tm_sec)
< 0)
{
*timebuf = '\0';
Expand Down
39 changes: 26 additions & 13 deletions nasl/nasl_misc_funcs.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#include <glib.h>
#include <gvm/util/compressutils.h> /* for gvm_uncompress */
#include <gvm/util/kb.h> /* for KB_TYPE_STR */
#include <stdbool.h> /* for boolean */
#include <stdlib.h> /* for lrand48 */
#include <string.h> /* for bzero */
#include <sys/time.h> /* for gettimeofday */
Expand Down Expand Up @@ -672,23 +673,35 @@ tree_cell *
nasl_localtime (lex_ctxt *lexic)
{
tree_cell *retc;
struct tm *ptm;
struct tm ptm;
time_t tictac;
int utc;
nasl_array *a;
anon_nasl_var v;
bool success;

tictac = get_int_var_by_num (lexic, 0, 0);
if (tictac == 0)
tictac = time (NULL);
utc = get_int_var_by_name (lexic, "utc", 0);

success = true;
if (utc)
ptm = gmtime (&tictac);
{
if (gmtime_r (&tictac, &ptm) == NULL)
{
success = false;
}
}
else
ptm = localtime (&tictac);
{
if (localtime_r (&tictac, &ptm) == NULL)
{
success = false;
}
}

if (ptm == NULL)
if (!success)
{
nasl_perror (lexic, "localtime(%d,utc=%d): %s\n", tictac, utc,
strerror (errno));
Expand All @@ -700,23 +713,23 @@ nasl_localtime (lex_ctxt *lexic)
memset (&v, 0, sizeof (v));
v.var_type = VAR2_INT;

v.v.v_int = ptm->tm_sec;
v.v.v_int = ptm.tm_sec;
add_var_to_array (a, "sec", &v); /* seconds */
v.v.v_int = ptm->tm_min;
v.v.v_int = ptm.tm_min;
add_var_to_array (a, "min", &v); /* minutes */
v.v.v_int = ptm->tm_hour;
v.v.v_int = ptm.tm_hour;
add_var_to_array (a, "hour", &v); /* hours */
v.v.v_int = ptm->tm_mday;
v.v.v_int = ptm.tm_mday;
add_var_to_array (a, "mday", &v); /* day of the month */
v.v.v_int = ptm->tm_mon + 1;
v.v.v_int = ptm.tm_mon + 1;
add_var_to_array (a, "mon", &v); /* month */
v.v.v_int = ptm->tm_year + 1900;
v.v.v_int = ptm.tm_year + 1900;
add_var_to_array (a, "year", &v); /* year */
v.v.v_int = ptm->tm_wday;
v.v.v_int = ptm.tm_wday;
add_var_to_array (a, "wday", &v); /* day of the week */
v.v.v_int = ptm->tm_yday + 1;
v.v.v_int = ptm.tm_yday + 1;
add_var_to_array (a, "yday", &v); /* day in the year */
v.v.v_int = ptm->tm_isdst;
v.v.v_int = ptm.tm_isdst;
add_var_to_array (a, "isdst", &v); /* daylight saving time */

return retc;
Expand Down
5 changes: 3 additions & 2 deletions nasl/nasl_var.c
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,7 @@ get_variable_by_name (lex_ctxt *ctxt, const char *name)
switch (t2)
{
case VAR2_INT:
nasl_trace (NULL, "NASL> %s <- %d\n", get_var_name (v1), v1->v.v_int);
nasl_trace (NULL, "NASL> %s <- %lu\n", get_var_name (v1), v1->v.v_int);
break;
case VAR2_STRING:
case VAR2_DATA:
Expand Down Expand Up @@ -876,7 +876,8 @@ get_variable_by_name (lex_ctxt *ctxt, const char *name)
ret->type = CONST_INT;
ret->x.i_val = v->v.v_int;
if (nasl_trace_enabled ())
nasl_trace (lexic, "NASL> %s -> %d\n", get_var_name (v), ret->x.i_val);
nasl_trace (lexic, "NASL> %s -> %lu\n", get_var_name (v),
ret->x.i_val);
return ret;

case VAR2_STRING:
Expand Down
5 changes: 4 additions & 1 deletion src/hosts.c
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,10 @@ host_set_time (kb_t kb, char *ip, char *type)
int len;

t = time (NULL);
timestr = g_strdup (ctime (&t));
char ts[26];
char *ts_ptr = ts;
ctime_r (&t, ts_ptr);
timestr = g_strdup (ts_ptr);
len = strlen (timestr);
if (timestr[len - 1] == '\n')
timestr[len - 1] = '\0';
Expand Down