Jump to content

maxxd

Gurus
  • Posts

    1,659
  • Joined

  • Last visited

  • Days Won

    52

Posts posted by maxxd

  1. Couple things right off the bat: first, if those are your actual database credentials, remove them from the post. Second, please use the code tags (the < > button on the post editor) when you post code - it makes it much easier to read. Third, the mysql_* functions have been deprecated for about a decade now and will be flat-out removed at some point in the near-ish future - use PDO (preferably) or mysqli_* instead. They both are still supported and let you use prepared statements, which will circumvent the massive security holes you've got in your current script. Google 'SQL Injection attacks' for more information. I'm assuming the spaces between the dollar sign and the variable name is a typo, as I don't think php will properly parse that, although it may not be noticeable if you're developing with errors turned off as you're assigning all the $_POST values to local variables and then ignoring those variables completely.

     

    Now, on to the meat of the question. If you've got the data saving (dangerously) to your database, you're halfway there, technically. On the account-tr.php page, you'll need to write a SELECT sql statement that pulls the newly inserted data from the database. So, on the initial page, after the insert completes successfully, you'll need to get the insert_id() and either store that in $_SESSION or pass it via the URL string (?id=xxxx). I'd recommend using sessions in this case, personally. Use that to select the row you just inserted, and you can print out the data on account-tr.php.

     

    Also, you're connecting to and selecting the same database twice - once in config.php, once in your processing script.

  2. Give this a shot and see if it gets you the information you're looking for. You may have to start with t_persons as your base table (as you have, I switched it to t_incidents because that seems to be the main data table but the join directions may cause some weirdness...)

    $qry = "SELECT	 p.PersonID    
    		,p.FamilyName
    		,p.FirstName
    		,p.OtherNames
    		,p.Gender
    		,p.ImagePath    
    		,i.IncidentID
    		,i.Incident
    		,i.IncidentDate
    		,s.StatusID
    		,ok.KeywordID
    		,c.Country
    		,a.Agency
    	FROM t_incidents i
    	LEFT JOIN t_persons p
    		ON i.PersonID = p.PersonID
    	LEFT JOIN t_countries c
    		ON i.CountryID = c.CountryID
    	LEFT JOIN t_status s
    		ON i.StatusID = s.StatusID
    	LEFT JOIN t_offenceskeyword ok
    		ON i.OffenseKeywordID = ok.KeywordID
    	LEFT JOIN t_Agencies a
    		ON i.AgencyID = a.AgencyID
    	WHERE p.PersonID = :pid";
    

    Basically, you don't need all the ID columns unless you plan on specifically doing something with them (sorting, filtering, etc.), and even if that is the case, you don't need to select them both. You're joining on the column value, so the values will be the same. Of course, as Barand pointed out, you're going to want to bind the $PersonID to the :pid parameter before you can actually run the query.

  3. To tell you the truth, if you're making the switch, I'd recommend PDO() over mysqli(). Despite the similarities in name, I've found it much easier to refactor old-school mysql_* code to PDO than mysqli(). And PDO is a bit more forward-looking as it's not specifically tied to one database type.

  4. Hey all.

     

    Obviously, the mysql_* functions are deprecated and have been for quite some time now, and will be removed soon. As of 5.5, using them should result in an E_DEPRECATED error, so it looks like we're getting closer to that happening. My question is - has anybody read or heard a reliable statement as to which future version will officially remove even legacy support for the functions? Like 5.x, 7.0, etc?

     

    A Google search isn't returning anything official so far as I can see, and I was curious.

  5. The typical cause was running with magic_quotes_gpc enabled which would automatically apply addslashes to all input data and then escaping the data again before putting it into the query without first undoing the magic quotes effect.

    I am so afraid that's the culprit. The server is run by the client, and unless they specifically turned on magic quotes, I'm a little concerned that php may have not been updated in however long. At which point I can just revert all the changes and let everything run on mysql_*, but still...

  6. Even with mysql_real_escape_string there should not be any kind of visible escaping after your data has entered the database. If you are seeing things like backslashes before quotes within the database when querying with phpMyAdmin then you're actually double-escaping the data which is incorrect.

     

    Using quote() as a replacement for mysql_real_escape_string is fine, however as suggested you should move to prepared statements as soon as you're able to. Note that unlike mysql_real_escape_string the quote method will add quotes around the string so you'll need to either update your queries to not include quotes or strip the leading and ending quotes.

    Yeah, I found out about quote() wrapping the string already and took care of that. And honestly, I didn't think I should see the escaping in the database - I'm wondering exactly what was done in the past that resulted in the issue to begin with. When doing my own code, I always used htmlentities() or htmlspecialchars(), so I was a bit confused. Thanks for the info!

     

    With the amount of re-write you will have to do to switch over to pdo, how can you NOT alter the queries at the same time?  You will simply remove the existing values and replace them with the placeholders and then modify the existing 'query()' call to 'execute(array(key=>value,key=>value))'.  Of course you have to reference the un-sanitized values but that too should be simple.

    I completely agree and I look forward to getting in there to do exactly that.

     

    Thanks for the info and advice, everybody - I'm going to mark this thread complete but feel free to keep the discussion going...

  7. I agree it would be better to do all at once, unfortunately that decision isn't up to me. Hopefully we'll get to it soon, but right now not so much. We've got in-line queries scattered all throughout the site that are already using the existing sanitization method. Obviously I don't want to just allow the user to insert unsanitized data directly into the queries, and I was hoping we could get by usig quote() until we can convince the client to let us redo the site entirely. At that point, it's prepared statements and not a worry for the future...

     

    I do use PDO and prepared statements, but this was a legacy piece that was inherited. The transition actually came about because I was tasked to handle some very minor maintenance on another part of the site and noticed the msql_* functions in the abstraction class. Brought it to the attention of the higher-ups and they were kind enough to listen and let me run with it despite the crazy backlog of work we're looking at.

  8. Hey y'all. Hopefully quick question on something  I've not come across before.

     

    I am doing a quick and dirty update on an existing site that is using the mysql_* functions to use PDO, and I'm wondering how much of a corollary there is between the mysq_real_escape_string() function and PDO::quote() method. We had a sanitization method that returned the submitted string after running mysql_real_escape_string() on it, and I've updated it to return the string after passing it through quote(). What I'm noticing in phpMyAdmin, though, is that new records inserted using the quote() sanitize don't encode quotes or add slashes or evidence any of the things that apparently mysql_real_escape_string() used to do (I always used a different scrub method in the past so I'm really not familiar with how it works under the hood).

     

    Is using quote() going to offer an equivalent level of protection against injection?

     

    Hopefully I'll get the go-ahead to take the time and revamp all the queries to use prepared statements, but right now that's not in the cards. At least the old site did abstract database interaction so I'm not chasing mysql_* functions all over the site...

     

    Any opinions and thoughts are very much welcome and thanks in advance!

  9. ignace is right - if class second extends class first, it's already got access to any public and protected methods and properties of the first class. So in your code, you would instantiate the second class, then call methods from either the first or second class. The only things you wouldn't have access to are private properties or methods, as you would expect. Inheritance is useful and easy to use, but also pretty heavily coupled. If you're trying to learn OOP in php, I'd recommend the book PHP Objects, Patterns, and Practice by Matt Zandstra. Personally, I found it an informative and fun read. Of course, I'm kind of a nerd, so fun is subjective, but hey.

  10. If you're going to load all the options at page load, you can output a select with <optgroup> tags. If you want to dynamically fill the drop-downs as your user makes selections (for instance, select 'Ontario' from the 'province' drop-down, then the separate 'cities' drop-down populates with the cities in Ontario - I've always called it cascading selection, but not sure if a Google search on that will result in anything as I may be the only person to refer to this setup that way...) you'll want to look at AJAX. Which may actually be easier as you don't have to store each individual selection between page loads because the page is only going to load once. So you just need to read the values of the form once the user fills out and submits the entire form.

  11. Couple things to add to mac_gyver's answer. First, ID in a table is typically (not always, so I'm making an assumption) an auto-increment field, which means you won't insert a value into that field at all. Drop it from the column and values lists. It also looks like you're missing the closing parenthesis in the values list of your query. Finally, strings need to be quoted, so $coment would be '$coment', etc. Turn on error reporting - it'll help.

  12. So what exactly is the purpose of the form? If it doesn't really do anything, why is it there? What is the not much that it does? For instance, a contact form may not insert any data into the database, but a malicious user could inject to: and from: headers into the comment section and use your contact form as a spam launcher to send mail to users. If the actual body of the spam mail contains malicious code, then yes. This could be a problem.

     

    I think what Jacques1 is saying is that any and all data needs to be carefully considered before anything is done with it, regardless of its eventual purpose - whether it's stored in the database, displayed to the current user, or e-mailed anywhere. I don't know that you need to validate the contents of the specific form element because I don't know the purpose of your form, but you should definitely handle all the other data - which I'm sure you're considering, given the fact that you've asked this question.

     

    Jacques1, please correct me if I'm wrong regarding my understanding of your post.

  13. You need to reconsider your approach. By trying to 'make things as simple as possible', you're twisting things way out of control. Consider using comboboxes for the your filters.

    <select name='type'>
        <option value='newcars'>New Cars</option>
        <option value='new'>New Whatever</option>
        <option value='newmake'>New Make</option>
    </select>
    <select name='year'>
        <option>2014</option>
        <option>2013</option>
        <option>2012</option>
    </select>
    <select name='color'>
        <option value='blue'>Blue</option>
        <option value='red'>Red</option>
        <option value='rusted'>Rusted Out</option>
    </select>
    

    Then you build it in php

    $link = "http://ebay.com/{$_GET['type']}/{$_GET['year']}/{$_GET['color']}/";
    

    I've honestly never used ebay so I don't know if that link makes any sense at all to it, but I think it's close to what you're trying to do?

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.