Skip to content
This repository has been archived by the owner on Jul 2, 2024. It is now read-only.

fixes issues found by static analysis tools #94

Merged
merged 4 commits into from
Aug 6, 2018
Merged

fixes issues found by static analysis tools #94

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ab27c7b
static analysis fixes
DonatJR Aug 2, 2018
7511d3f
static analysis fixex (tokenLib)
DonatJR Aug 2, 2018
f311957
merge develop
DonatJR Aug 5, 2018
b3ce0a8
static analysis fixes 2 (tokenLib)
bencikpeter Aug 6, 2018
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
2 changes: 1 addition & 1 deletion SharkCage/CageConfigurator/App.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
</startup>
<userSettings>
<CageConfigurator.Properties.Settings>
<setting name="PeristentKeepassPath" serializeAs="String">
<setting name="PersistentKeepassPath" serializeAs="String">
<value />
</setting>
</CageConfigurator.Properties.Settings>
Expand Down
111 changes: 60 additions & 51 deletions SharkCage/CageConfigurator/CageConfiguratorForm.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,26 +127,28 @@ private void applicationBrowseButton_Click(object sender, System.EventArgs e)
{
const string file_type = "*.exe";

var file_dialog = new OpenFileDialog();
file_dialog.CheckFileExists = true;
var install_dir = Registry.GetValue(REGISTRY_KEY, "InstallDir", "") as string;
file_dialog.InitialDirectory = install_dir;
file_dialog.Filter = $"Application|{file_type}";
var result = file_dialog.ShowDialog();
if (result == DialogResult.OK)
{
var chosen_file = file_dialog.FileName;
CheckPath(chosen_file, file_type, (string path) =>
using (var file_dialog = new OpenFileDialog())
{
file_dialog.CheckFileExists = true;
file_dialog.Filter = $"Application|{file_type}";
var install_dir = Registry.GetValue(REGISTRY_KEY, "InstallDir", "") as string;
file_dialog.InitialDirectory = install_dir;
var result = file_dialog.ShowDialog();
if (result == DialogResult.OK)
{
if (applicationPath.Text != path)
var chosen_file = file_dialog.FileName;
CheckPath(chosen_file, file_type, (string path) =>
{
applicationPath.Text = path;
SetUnsavedData(true);
}
if (applicationPath.Text != path)
{
applicationPath.Text = path;
SetUnsavedData(true);
}

applicationPath.SelectionStart = applicationPath.Text.Length;
applicationPath.ScrollToCaret();
});
applicationPath.SelectionStart = applicationPath.Text.Length;
applicationPath.ScrollToCaret();
});
}
}
}

Expand Down Expand Up @@ -196,16 +198,18 @@ private void openToolStripMenuItem_Click(object sender, EventArgs e)

const string file_type = ".sconfig";

var file_dialog = new OpenFileDialog();
file_dialog.CheckFileExists = true;
var folder = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.CommonDocuments), "SharkCage");
file_dialog.InitialDirectory = folder;
file_dialog.Filter = $"SharkCage configuration|*{file_type}";
var result = file_dialog.ShowDialog();
if (result == DialogResult.OK)
using (var file_dialog = new OpenFileDialog())
{
var chosen_file = file_dialog.FileName;
CheckPath(chosen_file, file_type, LoadConfig);
file_dialog.CheckFileExists = true;
file_dialog.Filter = $"SharkCage configuration|*{file_type}";
var folder = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.CommonDocuments), "SharkCage");
file_dialog.InitialDirectory = folder;
var result = file_dialog.ShowDialog();
if (result == DialogResult.OK)
{
var chosen_file = file_dialog.FileName;
CheckPath(chosen_file, file_type, LoadConfig);
}
}
}

Expand Down Expand Up @@ -257,7 +261,7 @@ private void LoadAdditionalApp(string additional_app, string additional_app_path
switch (additional_app)
{
case "Keepass":
Settings.Default.PeristentKeepassPath = additional_app_path;
Settings.Default.PersistentKeepassPath = additional_app_path;
secureSecondaryPrograms.SelectedIndex = 1;
break;
default:
Expand Down Expand Up @@ -295,16 +299,18 @@ private void secureSecondaryPrograms_SelectedIndexChanged(object sender, EventAr
switch (secureSecondaryPrograms.SelectedItem.ToString())
{
case "Keepass":
if (Settings.Default.PeristentKeepassPath.Length == 0)
if (Settings.Default.PersistentKeepassPath == String.Empty)
{
MessageBox.Show("There is no path saved for this program, please select it now.", "Shark Cage", MessageBoxButtons.OK, MessageBoxIcon.Information);
var keepass_file_dialog = new OpenFileDialog();
keepass_file_dialog.CheckFileExists = true;
keepass_file_dialog.Filter = "Keepass|Keepass.exe";
var result = keepass_file_dialog.ShowDialog();
if (result == DialogResult.OK)
using (var keepass_file_dialog = new OpenFileDialog())
{
Settings.Default.PeristentKeepassPath = keepass_file_dialog.FileName;
keepass_file_dialog.CheckFileExists = true;
keepass_file_dialog.Filter = "Keepass|Keepass.exe";
var result = keepass_file_dialog.ShowDialog();
if (result == DialogResult.OK)
{
Settings.Default.PersistentKeepassPath = keepass_file_dialog.FileName;
}
}
}
break;
Expand All @@ -324,24 +330,27 @@ private void tokenBrowseButton_Click(object sender, EventArgs e)
{
string[] file_types = { "*.bmp", "*.png", "*.jpeg", "*.jpg" };

var file_dialog = new OpenFileDialog();
file_dialog.CheckFileExists = true;
file_dialog.Filter = $"Picture|{String.Join(";", file_types)}";
var install_dir = Registry.GetValue(REGISTRY_KEY, "InstallDir", "") as string;
file_dialog.InitialDirectory = install_dir;
var result = file_dialog.ShowDialog();
if (result == DialogResult.OK)
using (var file_dialog = new OpenFileDialog())
{
var chosen_file = file_dialog.FileName;
CheckPath(chosen_file, file_types, (string path) =>
file_dialog.CheckFileExists = true;
file_dialog.Filter = $"Picture|{String.Join(";", file_types)}";
file_dialog.Filter = $"Picture|{String.Join(";", file_types)}";
var install_dir = Registry.GetValue(REGISTRY_KEY, "InstallDir", "") as string;
file_dialog.InitialDirectory = install_dir;
var result = file_dialog.ShowDialog();
if (result == DialogResult.OK)
{
if (tokenBox.ImageLocation != path)
var chosen_file = file_dialog.FileName;
CheckPath(chosen_file, file_types, (string path) =>
{
tokenBox.ImageLocation = path;
tokenBox.Load();
SetUnsavedData(true);
}
});
if (tokenBox.ImageLocation != path)
{
tokenBox.ImageLocation = path;
tokenBox.Load();
SetUnsavedData(true);
}
});
}
}
}

Expand Down Expand Up @@ -419,7 +428,7 @@ private void saveButton_Click(object sender, EventArgs e)
}

var secondary_path = GetSecondaryApplicationPath(secureSecondaryPrograms.Text);
if (secondary_path.Length == 0 && secureSecondaryPrograms.SelectedIndex != 0)
if (String.IsNullOrEmpty(secondary_path) && secureSecondaryPrograms.SelectedIndex != 0)
{
MessageBox.Show("You specified a secondary program but did not provide a corresponding location. Please reselect the application, provide the location and try again.",
"SharkCage", MessageBoxButtons.OK, MessageBoxIcon.Information);
Expand Down Expand Up @@ -549,7 +558,7 @@ private string GetSecondaryApplicationPath(string name)
switch (name)
{
case "Keepass":
return Settings.Default.PeristentKeepassPath;
return Settings.Default.PersistentKeepassPath;
default:
return String.Empty;
}
Expand Down
6 changes: 3 additions & 3 deletions SharkCage/CageConfigurator/Properties/Settings.Designer.cs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion SharkCage/CageConfigurator/Properties/Settings.settings
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<SettingsFile xmlns="http://schemas.microsoft.com/VisualStudio/2004/01/settings" CurrentProfile="(Default)" GeneratedClassNamespace="CageConfigurator.Properties" GeneratedClassName="Settings">
<Profiles />
<Settings>
<Setting Name="PeristentKeepassPath" Type="System.String" Scope="User">
<Setting Name="PersistentKeepassPath" Type="System.String" Scope="User">
<Value Profile="(Default)" />
</Setting>
</Settings>
Expand Down
2 changes: 1 addition & 1 deletion SharkCage/CageManager/FullWorkArea.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class FullWorkArea
~FullWorkArea();
bool Init();
private:
RECT rect;
RECT rect = { 0 };
bool GetBottomFromMonitor(int &monitor_bottom);
int cage_width;
};
8 changes: 8 additions & 0 deletions SharkCage/CageManager/SecuritySetup.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,19 @@ auto local_free_deleter = [&](T resource) { ::LocalFree(resource); };
class SecuritySetup
{
public:
SecuritySetup() {};

// these need to be customized if needed so security_descriptor also gets copied
// (otherwise there could be use-after-frees in the destructor)
SecuritySetup(const SecuritySetup &) = delete;
SecuritySetup& operator= (const SecuritySetup &) = delete;

~SecuritySetup()
{
if (security_descriptor)
{
::LocalFree(security_descriptor);
security_descriptor = nullptr;
}
}

Expand Down
19 changes: 14 additions & 5 deletions SharkCage/CageService/CageService.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ void CageService::StopCageManager()
std::wostringstream os;
os << "Terminated CageManager: 55" << std::endl;
::OutputDebugString(os.str().c_str());

::CloseHandle(cage_manager_handle);
}
else
{
Expand All @@ -187,20 +189,25 @@ std::wstring CageService::GetLastErrorAsString(DWORD error_id)
{
// Get the error message, if any.
LPWSTR message_buffer = nullptr;
size_t size = FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
size_t size = ::FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
NULL,
error_id,
MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
message_buffer,
0,
NULL);

std::wstring message(message_buffer, size);
if (message_buffer && size > 0)
{
std::wstring message(message_buffer, size);

// Free the buffer.
::LocalFree(message_buffer);
// Free the buffer.
::LocalFree(message_buffer);

return message;
return message;
}

return L"";
}

void CageService::HandleMessage(const std::wstring &message, NetworkManager &network_manager)
Expand Down Expand Up @@ -273,6 +280,8 @@ void CageService::HandleMessage(const std::wstring &message, NetworkManager &net
{
::CloseHandle(created_token);
}
::CloseHandle(cage_manager_handle);

cage_manager_process_id = 0;
}

Expand Down
1 change: 0 additions & 1 deletion SharkCage/CageServiceInstaller/ServiceInstaller.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

public static class ServiceInstaller
{
private const int STANDARD_RIGHTS_REQUIRED = 0xF0000;
private const int SERVICE_WIN32_OWN_PROCESS = 0x00000010;

[StructLayout(LayoutKind.Sequential)]
Expand Down
37 changes: 30 additions & 7 deletions SharkCage/SharedFunctionality/tokenLib/tokenLib.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,13 @@ class tokenTemplate {

public:
tokenTemplate(HANDLE &userToken);

~tokenTemplate();

// these need to be customized if needed so memory of member pointers also gets copied
// (otherwise there could be use-after-frees in the destructor)
tokenTemplate(const tokenTemplate &) = delete;
tokenTemplate operator=(const tokenTemplate &) = delete;

bool addGroup(PSID sid);

bool generateToken(HANDLE & token);
Expand Down Expand Up @@ -228,9 +232,17 @@ std::optional<std::vector<DWORD>> getProcessesWithBothPrivileges(const std::vect
{
HANDLE processHandle;
if ((processHandle = OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, FALSE, processPid)) == NULL) continue;
if (hasSeCreateTokenPrivilege(processHandle) && hasSeTcbPrivilege(processHandle))
try
{
processes.push_back(processPid);
if (hasSeCreateTokenPrivilege(processHandle) && hasSeTcbPrivilege(processHandle))
{
processes.push_back(processPid);
}
}
catch (const std::exception&)
{
CloseHandle(processHandle);
throw;
}
CloseHandle(processHandle);
}
Expand Down Expand Up @@ -332,9 +344,17 @@ std::optional<HANDLE> getProcessUnderLocalSystem(std::vector<DWORD> processes){
{
HANDLE processHandle;
if ((processHandle = OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, FALSE, processPid)) == NULL) continue;
if (processIsLocalSystem(processHandle))
try
{
if (processIsLocalSystem(processHandle))
{
return processHandle;
}
}
catch (const std::exception&)
{
return processHandle;
CloseHandle(processHandle);
throw;
}
CloseHandle(processHandle);
}
Expand Down Expand Up @@ -450,8 +470,11 @@ bool changePrivilege(bool privilegeStatus, LPCTSTR privilege) {
wprintf(L"Error getting token for privilege escalation\n");
return false;
}
return setPrivilege(userTokenHandle, privilege, privilegeStatus);

auto set_privilege_status = setPrivilege(userTokenHandle, privilege, privilegeStatus);

CloseHandle(userTokenHandle);
return set_privilege_status;
}

bool changeTokenCreationPrivilege(bool privilegeStatus) {
Expand Down Expand Up @@ -574,7 +597,7 @@ tokenTemplate::tokenTemplate(HANDLE &userToken) {
}

tokenTemplate::~tokenTemplate() {
delete objectAttributes->SecurityQualityOfService;
delete static_cast<PSECURITY_QUALITY_OF_SERVICE>(objectAttributes->SecurityQualityOfService);
delete objectAttributes;
delete authenticationId;
delete expirationTime;
Expand Down