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.

Sign up for GitHub

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

Jump to bottom

Make sure argv indeces exist before comparing them in test_lfs.c and test_time.c #91

Closed
wants to merge 1 commit into from
Closed

Make sure argv indeces exist before comparing them in test_lfs.c and test_time.c #91

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions src/f4/app_cantest/source/test_lfs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ void cmd_lfs(BaseSequentialStream *chp, int argc, char *argv[])
struct lfs_info info;
char buf[BUF_SIZE];

if (!strcmp(argv[0], "ls") && argc > 1) {
if (argv[0] != NULL && !strcmp(argv[0], "ls") && argc > 1) {
dir = dir_open(&FSD1, argv[1]);
if (dir == NULL) {
chprintf(chp, "Error in dir_open: %d\r\n", FSD1.err);
Expand Down Expand Up @@ -45,19 +45,19 @@ void cmd_lfs(BaseSequentialStream *chp, int argc, char *argv[])
return;
}
chprintf(chp, "\r\n");
} else if (!strcmp(argv[0], "mkdir") && argc > 1) {
} else if (argv[0] != NULL && !strcmp(argv[0], "mkdir") && argc > 1) {
ret = fs_mkdir(&FSD1, argv[1]);
if (ret < 0) {
chprintf(chp, "Error in fs_mkdir: %d\r\n");
return;
}
} else if (!strcmp(argv[0], "rm") && argc > 1) {
} else if (argv[0] != NULL && !strcmp(argv[0], "rm") && argc > 1) {
ret = fs_remove(&FSD1, argv[1]);
if (ret < 0) {
chprintf(chp, "Error in fs_remove: %d\r\n");
return;
}
} else if (!strcmp(argv[0], "cat") && argc > 1) {
} else if (argv[0] != NULL && !strcmp(argv[0], "cat") && argc > 1) {
file = file_open(&FSD1, argv[1], LFS_O_RDONLY);
if (file == NULL) {
chprintf(chp, "Error in file_open: %d\r\n", FSD1.err);
Expand All @@ -78,7 +78,7 @@ void cmd_lfs(BaseSequentialStream *chp, int argc, char *argv[])
chprintf(chp, "Error in file_close: %d\r\n", ret);
return;
}
} else if (!strcmp(argv[0], "hexdump") && argc > 1) {
} else if (argv[0] != NULL && !strcmp(argv[0], "hexdump") && argc > 1) {
file = file_open(&FSD1, argv[1], LFS_O_RDONLY);
if (file == NULL) {
chprintf(chp, "Error in file_open: %d\r\n", FSD1.err);
Expand All @@ -102,7 +102,7 @@ void cmd_lfs(BaseSequentialStream *chp, int argc, char *argv[])
chprintf(chp, "Error in file_close: %d\r\n", ret);
return;
}
} else if (!strcmp(argv[0], "load") && argc > 1) {
} else if (argv[0] != NULL && !strcmp(argv[0], "load") && argc > 1) {
uint8_t buf[BUF_SIZE] = {0};
char line[BUF_SIZE * 2] = {0};
char c, *p = line;
Expand Down Expand Up @@ -159,23 +159,23 @@ void cmd_lfs(BaseSequentialStream *chp, int argc, char *argv[])
chprintf(chp, "Error in file_close: %d\r\n", ret);
return;
}
} else if (!strcmp(argv[0], "mount")) {
} else if (argv[0] != NULL && !strcmp(argv[0], "mount")) {
chprintf(chp, "Attempting to mount LFS...\r\n");
ret = fs_mount(&FSD1, false);
if (ret < 0) {
chprintf(chp, "Mount failed: %d\r\n", ret);
return;
}
chprintf(chp, "OK\r\n");
} else if (!strcmp(argv[0], "unmount")) {
} else if (argv[0] != NULL && !strcmp(argv[0], "unmount")) {
chprintf(chp, "Attempting to unmount LFS...\r\n");
ret = fs_unmount(&FSD1);
if (ret < 0) {
chprintf(chp, "Unmount failed: %d\r\n", ret);
return;
}
chprintf(chp, "OK\r\n");
} else if (!strcmp(argv[0], "format")) {
} else if (argv[0] != NULL && !strcmp(argv[0], "format")) {
chprintf(chp, "Attempting to format LFS...\r\n");
ret = fs_format(&FSD1);
if (ret < 0) {
Expand Down
16 changes: 8 additions & 8 deletions src/f4/app_cantest/source/test_time.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,42 +16,42 @@ void cmd_time(BaseSequentialStream *chp, int argc, char *argv[])
if (argc < 1) {
goto time_usage;
}
if (!strcmp(argv[0], "unix")) {
if (!strcmp(argv[1], "get")) {
if (argv[0] != NULL && !strcmp(argv[0], "unix")) {
if (argv[1] != NULL && !strcmp(argv[1], "get")) {
time_t unix_time = rtcGetTimeUnix(&msec);
char *timestr = ctime(&unix_time);
chprintf(chp, "UNIX Time: %d\r\n"
"Date: %s\r\n",
unix_time, timestr);
} else if (!strcmp(argv[1], "set") && argc > 2) {
} else if (argv[1] != NULL && !strcmp(argv[1], "set") && argc > 2) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this (and similar) situations this actually creates a new subtle bug. You're checking if the value at the location is zero'ed out- which sometimes will be true- sometimes wont. This also assumes that this is in-bounds.

Me thinks it would be better to check against argc and ensure it is greater than or equal to the current spot you are trying to deference.

For example: argv[0] is technically always a valid pointer, but it's also a de-reference operation that's not guaranteed to work if the array was never allocated, or not allocated with the size you assume. This code would break if we simply passed no args to this tool.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the description for now, will make it more detailed ASAP. Will also make this conditional/similar more sturdy, likely today or tomorrow.
Thanks!

rtcSetTimeUnix(strtoul(argv[2], NULL, 0), 0);
} else {
goto time_usage;
}
} else if (!strcmp(argv[0], "scet")) {
} else if (argv[0] != NULL && !strcmp(argv[0], "scet")) {
if (!strcmp(argv[1], "get")) {
rtcGetTimeSCET(&scet);
chprintf(chp, "SCET Time: %u.%u\r\n", scet.coarse, scet.fine);
} else if (!strcmp(argv[1], "set") && argc > 3) {
} else if (argv[1] != NULL && !strcmp(argv[1], "set") && argc > 3) {
scet.coarse = strtoul(argv[2], NULL, 0);
scet.fine = strtoul(argv[3], NULL, 0);
rtcSetTimeSCET(&scet);
} else {
goto time_usage;
}
} else if (!strcmp(argv[0], "utc")) {
} else if (argv[0] != NULL && !strcmp(argv[0], "utc")) {
if (!strcmp(argv[1], "get")) {
rtcGetTimeUTC(&utc);
chprintf(chp, "UTC Time: Day: %u ms: %u us: %u\r\n", utc.day, utc.ms, utc.us);
} else if (!strcmp(argv[1], "set") && argc > 4) {
} else if (argv[1] != NULL && !strcmp(argv[1], "set") && argc > 4) {
utc.day = strtoul(argv[2], NULL, 0);
utc.ms = strtoul(argv[3], NULL, 0);
utc.us = strtoul(argv[4], NULL, 0);
rtcSetTimeUTC(&utc);
} else {
goto time_usage;
}
} else if (!strcmp(argv[0], "raw")) {
} else if (argv[1] != NULL && !strcmp(argv[0], "raw")) {
rtcGetTime(&RTCD1, &timespec);
chprintf(chp, "Year: %u Month: %u DST: %u DoW: %u Day: %u ms: %u\r\n",
timespec.year, timespec.month, timespec.dstflag, timespec.dayofweek, timespec.day, timespec.millisecond);
Expand Down
Loading