Jump to content


Photo

almost done


  • This topic is locked This topic is locked
13 replies to this topic

#1 jcombs_31

jcombs_31
  • Staff Alumni
  • Advanced Member
  • 2,066 posts
  • LocationFL

Posted 04 September 2006 - 12:56 AM

I have some more things to do, but most of this site is done for the citrus/landscape/racing site.

http://www.a1acitrus...m/new/index.php

What do you think...?

#2 AndyB

AndyB
  • Staff Alumni
  • Advanced Member
  • 5,465 posts
  • LocationToronto

Posted 04 September 2006 - 01:37 AM

It's excellent. Three tiny things - the mouseOver color on the side menu links are a little light. Although I'm sure they exactly match a colour used elsewhere ...

A link to 'top of page' might be worthwhile on the longer pages.

And the other is a personal piece of weirdness. On the Citrus and 3D Landscape pages, the side menu ends with a sort of 'quote' graphic with a large point end that's directing me away from the content. Visually, it might be better to make it point back at the page content.
Legend has it that reading the manual never killed anyone.
My site

#3 bob_the _builder

bob_the _builder
  • Members
  • PipPipPip
  • Advanced Member
  • 207 posts

Posted 04 September 2006 - 05:43 AM

Hi,

Just 1 thing ... your contact form, it clears the form when submited with missing fields, you should echo the post data in the fields so they dont have to retype the data if they miss a field out.

Nice job!

#4 steviewdr

steviewdr
  • Moderators
  • Advanced Member
  • 1,364 posts
  • LocationIreland

Posted 04 September 2006 - 09:14 AM

Looks very well.

1 thing however, your Citrus page - There is a PERFECT crisp image of a citrus in the top image on the left. This is a perfect quality image.
When comparing the quality of this image to that on the index page - of the orange, the orange isn't of similar quality. I think you need a better orange image for the index page for the top image.

It is VERY sweet apart from that - a HUGE improvement on their current website ,-)

Oh -
http://www.a1acitrus-racing.com/new/landscape.php?id='858
http://www.a1acitrus-racing.com/new/citrus.php?id='14
Be careful!!


-steve


#5 448191

448191
  • Staff Alumni
  • Advanced Member
  • 3,545 posts
  • LocationNetherlands

Posted 04 September 2006 - 11:06 AM

Oh -
http://www.a1acitrus-racing.com/new/landscape.php?id='858
http://www.a1acitrus-racing.com/new/citrus.php?id='14
Be careful!!


I think he's already using mysql_real_escape_string, so above examples will never produce anything usefull for a hacker.

I tried a little hacking of my own, and although I'm not an expert by any standard, I did find that there is some small room for exploitation.

http://www.a1acitrus...php?id=1 or 1=1

Gives me all items. I know that's not very scary. ;D

Anything with quotes failed, but I was able to probe for the table name.

http://www.a1acitrus....id IS NOT NULL

Could not run query: Unknown table 'citrus_prod' in where clause


I didn't find it though..

When trying to get table names, I did get some of the MySQL connection (user/host) information:
http://www.a1acitrus...N_SCHEMA.TABLES

Could not run query: SELECT command denied to user: 'jcombs31@linhost114.mesa1.secureserver.net' for table 'TABLES'


Any subqueries I tried all failed.

All of this isn't very concerning, but I would still recommend you validate the string in $_GET['id'] to be numeric. That will eliminate any room for exploitation there might be.

Edit: Just a question, did you secure your contact form against email header injection? I would've tried it out, but didn't want to bother your client with obnoxious emails...

#6 jcombs_31

jcombs_31
  • Staff Alumni
  • Advanced Member
  • 2,066 posts
  • LocationFL

Posted 04 September 2006 - 02:53 PM

Oh -
http://www.a1acitrus-racing.com/new/landscape.php?id='858
http://www.a1acitrus-racing.com/new/citrus.php?id='14
Be careful!!


I think he's already using mysql_real_escape_string, so above examples will never produce anything usefull for a hacker.

I tried a little hacking of my own, and although I'm not an expert by any standard, I did find that there is some small room for exploitation.

http://www.a1acitrus...php?id=1 or 1=1

Gives me all items. I know that's not very scary. ;D

Anything with quotes failed, but I was able to probe for the table name.

http://www.a1acitrus....id IS NOT NULL

Could not run query: Unknown table 'citrus_prod' in where clause


I didn't find it though..

When trying to get table names, I did get some of the MySQL connection (user/host) information:
http://www.a1acitrus...N_SCHEMA.TABLES

Could not run query: SELECT command denied to user: 'jcombs31@linhost114.mesa1.secureserver.net' for table 'TABLES'


Any subqueries I tried all failed.

All of this isn't very concerning, but I would still recommend you validate the string in $_GET['id'] to be numeric. That will eliminate any room for exploitation there might be.

Edit: Just a question, did you secure your contact form against email header injection? I would've tried it out, but didn't want to bother your client with obnoxious emails...


Thanks for the feedback.  How would you suggest securing the email for against header injection?

Hi,

Just 1 thing ... your contact form, it clears the form when submited with missing fields, you should echo the post data in the fields so they dont have to retype the data if they miss a field out.

Nice job!



I know, didn't finish the contact form yet, just wanted design critiques mostly, but I'll finish that up today probably.

It's excellent. Three tiny things - the mouseOver color on the side menu links are a little light. Although I'm sure they exactly match a colour used elsewhere ...

A link to 'top of page' might be worthwhile on the longer pages.

And the other is a personal piece of weirdness. On the Citrus and 3D Landscape pages, the side menu ends with a sort of 'quote' graphic with a large point end that's directing me away from the content. Visually, it might be better to make it point back at the page content.


Thanks Andy, nice subtle point about the pointer.

#7 448191

448191
  • Staff Alumni
  • Advanced Member
  • 3,545 posts
  • LocationNetherlands

Posted 04 September 2006 - 08:08 PM

Thanks for the feedback.  How would you suggest securing the email for against header injection?


Validate. All fields other than the body cannot contain "\r", "\n", "%0D" or "%0D". I don't believe there is a way to inject malicous MIME stuff into the body if the other fields are secure, since php mail() already created the top-level part of the email. Make sure you disable multiparts by including a "Content-Type: text/plain;" header. I could be wrong, so maybe you should check for 'Content-Type' in the body. Make sure there is no HTML or javascript in there, to be sure.

#8 jcombs_31

jcombs_31
  • Staff Alumni
  • Advanced Member
  • 2,066 posts
  • LocationFL

Posted 05 September 2006 - 12:34 PM

Yea, I was reading more about it, believe it or not, i have not really been that careful with email forms before, but will update  my class for more protection.

BTW, I never send it to the client email while in testing, so feel free to abuse it if you wish.

#9 jcombs_31

jcombs_31
  • Staff Alumni
  • Advanced Member
  • 2,066 posts
  • LocationFL

Posted 06 September 2006 - 09:04 PM

I think I fixed any problems you guys mentioned, with the exception of the item andy mentioned below the sub-menus. (I"m a little lazy for that now).  Any final comments?  Just a couple small things I have to finish, such as the slideshow on the racing pages, and a small CMS for them to update inventory.

#10 steviewdr

steviewdr
  • Moderators
  • Advanced Member
  • 1,364 posts
  • LocationIreland

Posted 07 September 2006 - 09:44 AM

Just a few things:

The graphic saying "Delivery Available" - I think you should make it a link to a "Delivery" section with info on the rates etc. IMO.

For the Racing - and Car specifications - I would prefer to see a graphic of a racing car, rather than the driver again.

Thats about it. Looking forward to see the image gallery.

-steve

#11 jcombs_31

jcombs_31
  • Staff Alumni
  • Advanced Member
  • 2,066 posts
  • LocationFL

Posted 07 September 2006 - 11:09 AM

Stevie, they want everyone to call for rates for the most part because it will vary.  I somewhat agree about the specs page, but the car image would not stretch like the image of him, but I'll maybe try it out and see how it looks.

I don't think the image gallery will be anything special, I'm going to try to piece it together from my own image gallery that i created.

#12 steviewdr

steviewdr
  • Moderators
  • Advanced Member
  • 1,364 posts
  • LocationIreland

Posted 08 September 2006 - 11:57 AM

The image gallery ->

You need to specify a size for the main Div for the photo. It works PERFECT when the images are cached by my browser. BUT going through the image gallery for the first time (delete your temp internet files and see) the image page jumps up and down when loading the image.

Your image gallery link on http://www.a1acitrus...ing.php?id=kids is broken.

Coming along nicely.

-steve

#13 dk4210

dk4210
  • Members
  • PipPipPip
  • Advanced Member
  • 154 posts
  • LocationRoswell, GA

Posted 08 September 2006 - 02:46 PM

I think that you site has a very clean layout. Is this a CMS?
Thanks, Dan
Kelly Web Services
Sales@kellywebserv.com

#14 jcombs_31

jcombs_31
  • Staff Alumni
  • Advanced Member
  • 2,066 posts
  • LocationFL

Posted 08 September 2006 - 02:59 PM

I think that you site has a very clean layout. Is this a CMS?
Thanks, Dan


no




0 user(s) are reading this topic

0 members, 0 guests, 0 anonymous users