Memory Access Violation

[Sumitsahu896]

Project: Shell

Report By: Sumit Sahu - Team Red

Vulnerability Type: Format String Vulnerability causing Memory Access Violation and Buffer over-read

Exploitability: Yes, can read memory parts that are not intended to, causing a breach in the system

Analysis Method Used: Flaw finder and Manual

Vulnerability Description:

util.cpp : 145, 152, 158 (strncpy)

memset(cmd,'\0',MAX_LENGTH);
            strncpy(cmd,cmdLine,strlen(cmdLine));

            tokens = strtok((char*)cmd, " ");            
            commands->cmd_count = 1;
            commands->cmds[index]->argc = count-1;

            strncpy(commands->cmds[index]->cmd_name,tokens,strlen(tokens));

            for(int i=0; i<count-1; i++)
            {    
                tokens = strtok(NULL, " ");          
                strncpy(commands->cmds[index]->argument[i],tokens,strlen(tokens));

            }

Description

strcpy depends on a trailing \0, which may not always occur. This might result in getting more inputs (or tokens and commands in this case) and will mess up the desired out. The adversary might result in using more than required commands to get the list of files or trigger more vulnerable commands. And since this piece of code accepts multiple commands, it might act as a catalyst for the breach.

Possible Fix:

For a "saferstrcpy()", you are better off using strncat() like so:

 if (dest_size > 0)
{
    dest[0] = '\0';
    strncat(dest, source, dest_size - 1);
}

That will always nul-terminate the result, and won't copy more than necessary.

[mustakimur]

@DamoSWL @shawnwork @QuuikSilva98 please, review the bug report and confirm the criticality. Also, patch the fix to resolve the issue.

[mustakimur]

@DamoSWL you have to explain before you close an issue. This was very rude.

[DamoSWL]

this has been solved