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

SQL injection attack possiblity #3

Closed
toddejohnson opened this issue Apr 13, 2016 · 1 comment
Closed

SQL injection attack possiblity #3

toddejohnson opened this issue Apr 13, 2016 · 1 comment

Comments

@toddejohnson
Copy link

There's a SQL Injection possiblity because $_GET['report'] isn't validated to be numeric or escaped.

echo tmpl_page( ""
    .tmpl_reportList($allowed_reports)
    .tmpl_reportData( (isset($_GET["report"]) ? $_GET["report"] : false ), $allowed_reports )
);
if (!$reportnumber) {
    return "";
}
$sql = "SELECT * FROM rptrecord where serial = $reportnumber";

I'd suggest adding validation:

if(isset($_GET['report']) && is_numeric($_GET['report'])){
        $reportid=$_GET['report'];
}elseif(!isset($_GET['report'])){
        $reportid=false;
}else{
        die('Invalid Report ID');
}
// Generate Page with report list and report data (if a report is selected).
echo tmpl_page( ""
        .tmpl_reportList($allowed_reports)
        .tmpl_reportData( $reportid, $allowed_reports )
);

Patch attached.
dmarcts-report-viewer.txt

techsneeze added a commit that referenced this issue Apr 13, 2016
@techsneeze
Copy link
Owner

Thanks for the suggestion and the patch! The patch has been applied and posted!

techsneeze pushed a commit that referenced this issue Oct 2, 2017
a bit fancy stuff and make domain selectable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants