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

feat: /fopen command to auto-open downloaded files #273

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from
3 changes: 2 additions & 1 deletion src/chat.c
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,10 @@ static const char *chat_cmd_list[] = {
#ifdef QRCODE
"/myqr",
#endif /* QRCODE */
"/fopen",
"/nick",
"/note",
"/nospam",
"/note",
"/quit",
"/savefile",
"/sendfile",
Expand Down
113 changes: 113 additions & 0 deletions src/chat_commands.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@
*
*/

#include <dirent.h>
#include <stdlib.h>
#include <string.h>
#include <stdio.h>

#include "chat.h"
#include "conference.h"
Expand Down Expand Up @@ -335,6 +337,117 @@

#endif // GAMES


void cmd_fopen(WINDOW *window, ToxWindow *self, Tox *tox, int argc, char (*argv)[MAX_STR_SIZE])
{
UNUSED_VAR(window);

if (argc < 1) {
line_info_add(self, false, NULL, NULL, SYS_MSG, 0, 0, "File ID required.");
return;
}

long int idx = strtol(argv[1], NULL, 10);

if ((idx == 0 && strcmp(argv[1], "0")) || idx < 0 || idx >= MAX_FILES) {
line_info_add(self, false, NULL, NULL, SYS_MSG, 0, 0, "No pending file transfers with ID %ld", idx);
return;
}

FileTransfer *ft = get_file_transfer_struct_index(self->num, idx, FILE_TRANSFER_RECV);

if (!ft) {
line_info_add(self, false, NULL, NULL, SYS_MSG, 0, 0, "No pending file transfers with ID %ld.", idx);
return;
}

if (ft->state != FILE_TRANSFER_PENDING) {
line_info_add(self, false, NULL, NULL, SYS_MSG, 0, 0, "No pending file transfers with ID %ld.", idx);
return;
}

if ((ft->file = fopen(ft->file_path, "a")) == NULL) {
const char *msg = "File transfer failed: Invalid download path.";
close_file_transfer(self, tox, ft, TOX_FILE_CONTROL_CANCEL, msg, notif_error);
return;
}

Tox_Err_File_Control err;
tox_file_control(tox, self->num, ft->filenumber, TOX_FILE_CONTROL_RESUME, &err);

if (err != TOX_ERR_FILE_CONTROL_OK) {
goto on_recv_error;
}

// make tmp_dir if it does not exist
/// don't need this for now, prob should go somewhere else anyway.
const char *tmp_dir = "/tmp/toxic-download-dir/";
DIR* dir = opendir("/tmp/toxic-download-dir/");
if (dir) {
closedir(dir);
}

if (mkdir(tmp_dir, S_IRWXU) == -1) {
line_info_add(self, false, NULL, NULL, SYS_MSG, 0, 0, "Could not create tox download /tmp/ directory.\n", err);
}
// end make tmpdir if it does not exist
Copy link
Member

Choose a reason for hiding this comment

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

i am not sure what the reasoning behind the temp dir is.


line_info_add(self, false, NULL, NULL, SYS_MSG, 0, 0, "The file path is '%s'", ft->file_path);

line_info_add(self, false, NULL, NULL, SYS_MSG, 0, 0, "Saving file [%ld] as: '%s'", idx, ft->file_path);

const bool auto_accept_files = friend_get_auto_accept_files(self->num);
const uint32_t line_skip = auto_accept_files ? 4 : 2;

char progline[MAX_STR_SIZE];
init_progress_bar(progline);
line_info_add(self, false, NULL, NULL, SYS_MSG, 0, 0, "%s", progline);
ft->line_id = self->chatwin->hst->line_end->id + line_skip;
ft->state = FILE_TRANSFER_STARTED;

// make and call xdg command
while (ft->state != FILE_TRANSFER_INACTIVE) {
usleep(100000);
}

Check warning on line 411 in src/chat_commands.c

View check run for this annotation

codefactor.io / CodeFactor

src/chat_commands.c#L411

Possible OS Command Injection (CWE-78) (flawfinder7-execl)
Copy link
Member

Choose a reason for hiding this comment

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

there are legitimate uses for opening files that are still being transferred (most streaming media types like videos and audio)

Copy link
Member

Choose a reason for hiding this comment

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

That sleep will freeze toxic. ft->state can't update while we're in that loop because that function is holding a mutex lock. Sleeps in random functions is almost never the correct solution. They're generally only used in the main loop or in task-specific threads.

char command[MAX_STR_SIZE];
snprintf(command, sizeof(command), "xdg-open %s", ft->file_path);
Copy link
Member

@Green-Sky Green-Sky Jan 13, 2024

Choose a reason for hiding this comment

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

to mitigate potential command injection, i suggest we sanitize the string and make them url encoded (with the exception of '/')
eg: xdg-open file:///home/green/mpv-shot0001.jpg
or with space: xdg-open file:///home/green/mpv%20shot0001.jpg

Copy link
Member

Choose a reason for hiding this comment

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

the url encoder could be used for other hyperlinks too.

image

line_info_add(self, false, NULL, NULL, SYS_MSG, 0, 0, "I am about to run the xdg command.");
line_info_add(self, false, NULL, NULL, SYS_MSG, 0, 0, "The file path is '%s'", ft->file_path);

int open_result = popen(command, "r");
Copy link
Member

@nurupo nurupo Jan 13, 2024

Choose a reason for hiding this comment

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

popen() is a rather dangerous function that does shell interpretation, so it's prone to shell injection. Why not fork() + exec*() instead, calling /usr/bin/xdg-open directly?

Copy link
Member

Choose a reason for hiding this comment

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

while I agree with the first statement, said path is not portable.
nixos:

$ which xdg-open
/run/current-system/sw/bin/xdg-open

Copy link
Member

Choose a reason for hiding this comment

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

ie we would have to search for the binary first (probably once at startup)


if (open_result == -1) {
line_info_add(self, false, NULL, NULL, SYS_MSG, 0, 0, "Could not open file.");
}
// end make and call xdg command

return;

on_recv_error:

switch (err) {
case TOX_ERR_FILE_CONTROL_FRIEND_NOT_FOUND:
line_info_add(self, false, NULL, NULL, SYS_MSG, 0, 0, "File transfer failed: Friend not found.");
return;

case TOX_ERR_FILE_CONTROL_FRIEND_NOT_CONNECTED:
line_info_add(self, false, NULL, NULL, SYS_MSG, 0, 0, "File transfer failed: Friend is not online.");
return;

case TOX_ERR_FILE_CONTROL_NOT_FOUND:
line_info_add(self, false, NULL, NULL, SYS_MSG, 0, 0, "File transfer failed: Invalid filenumber.");
return;

case TOX_ERR_FILE_CONTROL_SENDQ:
line_info_add(self, false, NULL, NULL, SYS_MSG, 0, 0, "File transfer failed: Connection error.");
return;

default:
line_info_add(self, false, NULL, NULL, SYS_MSG, 0, 0, "File transfer failed (error %d)\n", err);
return;
}
}

void cmd_savefile(WINDOW *window, ToxWindow *self, Tox *tox, int argc, char (*argv)[MAX_STR_SIZE])
{
UNUSED_VAR(window);
Expand Down
3 changes: 2 additions & 1 deletion src/chat_commands.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@ void cmd_autoaccept_files(WINDOW *window, ToxWindow *self, Tox *tox, int argc, c
void cmd_cancelfile(WINDOW *window, ToxWindow *self, Tox *tox, int argc, char (*argv)[MAX_STR_SIZE]);
void cmd_conference_invite(WINDOW *, ToxWindow *, Tox *, int argc, char (*argv)[MAX_STR_SIZE]);
void cmd_conference_join(WINDOW *, ToxWindow *, Tox *, int argc, char (*argv)[MAX_STR_SIZE]);
void cmd_fopen(WINDOW *, ToxWindow *, Tox *, int argc, char (*argv)[MAX_STR_SIZE]);
void cmd_game_join(WINDOW *, ToxWindow *, Tox *, int argc, char (*argv)[MAX_STR_SIZE]);
void cmd_group_accept(WINDOW *window, ToxWindow *self, Tox *tox, int argc, char (*argv)[MAX_STR_SIZE]);
void cmd_group_invite(WINDOW *window, ToxWindow *self, Tox *tox, int argc, char (*argv)[MAX_STR_SIZE]);
void cmd_game_join(WINDOW *, ToxWindow *, Tox *, int argc, char (*argv)[MAX_STR_SIZE]);
void cmd_savefile(WINDOW *, ToxWindow *, Tox *, int argc, char (*argv)[MAX_STR_SIZE]);
void cmd_sendfile(WINDOW *, ToxWindow *, Tox *, int argc, char (*argv)[MAX_STR_SIZE]);

Expand Down
1 change: 1 addition & 0 deletions src/execute.c
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ static struct cmd_func chat_commands[] = {
#ifdef GAMES
{ "/play", cmd_game_join },
#endif
{ "/fopen", cmd_fopen },
{ "/savefile", cmd_savefile },
{ "/sendfile", cmd_sendfile },
#ifdef AUDIO
Expand Down